Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make required dependent columns in shared table non-nullable. #21952

Merged
merged 1 commit into from
Aug 6, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
138 changes: 138 additions & 0 deletions src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -137,6 +137,16 @@ protected virtual void GenerateEntityTypes(

GenerateEntityTypeRelationships(builderName, entityType, stringBuilder);
}

foreach (var entityType in entityTypes.Where(
e => !e.HasDefiningNavigation()
&& e.FindOwnership() == null
&& e.GetDeclaredNavigations().Any(n => !n.IsOnDependent && !n.ForeignKey.IsOwnership)))
{
stringBuilder.AppendLine();

GenerateEntityTypeNavigations(builderName, entityType, stringBuilder);
}
}

/// <summary>
Expand Down Expand Up @@ -215,6 +225,9 @@ protected virtual void GenerateEntityType(
if (ownerNavigation != null)
{
GenerateRelationships(builderName, entityType, stringBuilder);

GenerateNavigations(builderName, entityType.GetDeclaredNavigations()
.Where(n => !n.IsOnDependent && !n.ForeignKey.IsOwnership), stringBuilder);
}

GenerateData(builderName, entityType.GetProperties(), entityType.GetSeedData(providerValues: true), stringBuilder);
Expand Down Expand Up @@ -318,6 +331,9 @@ protected virtual void GenerateRelationships(
GenerateForeignKeys(builderName, entityType.GetDeclaredForeignKeys(), stringBuilder);

GenerateOwnedTypes(builderName, entityType.GetDeclaredReferencingForeignKeys().Where(fk => fk.IsOwnership), stringBuilder);

GenerateNavigations(builderName, entityType.GetDeclaredNavigations()
.Where(n => n.IsOnDependent || (!n.IsOnDependent && n.ForeignKey.IsOwnership)), stringBuilder);
}

/// <summary>
Expand Down Expand Up @@ -1188,6 +1204,128 @@ protected virtual void GenerateForeignKeyAnnotations(
GenerateAnnotations(annotations.Values, stringBuilder);
}

/// <summary>
/// Generates code for the navigations of an <see cref="IEntityType" />.
/// </summary>
/// <param name="builderName"> The name of the builder variable. </param>
/// <param name="entityType"> The entity type. </param>
/// <param name="stringBuilder"> The builder code is added to. </param>
protected virtual void GenerateEntityTypeNavigations(
[NotNull] string builderName,
[NotNull] IEntityType entityType,
[NotNull] IndentedStringBuilder stringBuilder)
{
Check.NotEmpty(builderName, nameof(builderName));
Check.NotNull(entityType, nameof(entityType));
Check.NotNull(stringBuilder, nameof(stringBuilder));

stringBuilder
.Append(builderName)
.Append(".Entity(")
.Append(Code.Literal(entityType.Name))
.AppendLine(", b =>");

using (stringBuilder.Indent())
{
stringBuilder.Append("{");

using (stringBuilder.Indent())
{
GenerateNavigations("b", entityType.GetDeclaredNavigations()
.Where(n => !n.IsOnDependent && !n.ForeignKey.IsOwnership), stringBuilder);
}

stringBuilder.AppendLine("});");
}
}

/// <summary>
/// Generates code for <see cref="INavigation" /> objects.
/// </summary>
/// <param name="builderName"> The name of the builder variable. </param>
/// <param name="navigations"> The navigations. </param>
/// <param name="stringBuilder"> The builder code is added to. </param>
protected virtual void GenerateNavigations(
[NotNull] string builderName,
[NotNull] IEnumerable<INavigation> navigations,
[NotNull] IndentedStringBuilder stringBuilder)
{
Check.NotNull(builderName, nameof(builderName));
Check.NotNull(navigations, nameof(navigations));
Check.NotNull(stringBuilder, nameof(stringBuilder));

foreach (var navigation in navigations)
{
stringBuilder.AppendLine();

GenerateNavigation(builderName, navigation, stringBuilder);
}
}

/// <summary>
/// Generates code for an <see cref="INavigation" />.
/// </summary>
/// <param name="builderName"> The name of the builder variable. </param>
/// <param name="navigation"> The navigation. </param>
/// <param name="stringBuilder"> The builder code is added to. </param>
protected virtual void GenerateNavigation(
[NotNull] string builderName,
[NotNull] INavigation navigation,
[NotNull] IndentedStringBuilder stringBuilder)
{
Check.NotNull(builderName, nameof(builderName));
Check.NotNull(navigation, nameof(navigation));
Check.NotNull(stringBuilder, nameof(stringBuilder));

stringBuilder
.Append(builderName)
.Append(".Navigation(")
.Append(Code.Literal(navigation.Name))
.Append(")");

using (stringBuilder.Indent())
{
if (!navigation.IsOnDependent
&& !navigation.IsCollection
&& navigation.ForeignKey.IsRequiredDependent)
{
stringBuilder
.AppendLine()
.Append(".IsRequired()");
}

GenerateNavigationAnnotations(navigation, stringBuilder);
}

stringBuilder.AppendLine(";");
}

/// <summary>
/// Generates code for the annotations on a navigation.
/// </summary>
/// <param name="navigation"> The navigation. </param>
/// <param name="stringBuilder"> The builder code is added to. </param>
protected virtual void GenerateNavigationAnnotations(
[NotNull] INavigation navigation, [NotNull] IndentedStringBuilder stringBuilder)
{
Check.NotNull(navigation, nameof(navigation));
Check.NotNull(stringBuilder, nameof(stringBuilder));

var annotations = Dependencies.AnnotationCodeGenerator
.FilterIgnoredAnnotations(navigation.GetAnnotations())
.ToDictionary(a => a.Name, a => a);

foreach (var methodCallCodeFragment in
Dependencies.AnnotationCodeGenerator.GenerateFluentApiCalls(navigation, annotations))
{
stringBuilder
.AppendLine()
.Append(Code.Fragment(methodCallCodeFragment));
}

GenerateAnnotations(annotations.Values, stringBuilder);
}

/// <summary>
/// Generates code for annotations.
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
using Microsoft.EntityFrameworkCore.Design;
using Microsoft.EntityFrameworkCore.Design.Internal;
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Internal;
using Microsoft.EntityFrameworkCore.Metadata;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Utilities;
Expand Down
66 changes: 64 additions & 2 deletions src/EFCore.Relational/Design/AnnotationCodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -214,15 +214,37 @@ public virtual IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(

/// <inheritdoc />
public virtual IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
IForeignKey foreignKey, IDictionary<string, IAnnotation> annotations)
IForeignKey navigation, IDictionary<string, IAnnotation> annotations)
AndriySvyryd marked this conversation as resolved.
Show resolved Hide resolved
{
var methodCallCodeFragments = new List<MethodCallCodeFragment>();

GenerateSimpleFluentApiCall(
annotations,
RelationalAnnotationNames.Name, nameof(RelationalForeignKeyBuilderExtensions.HasConstraintName), methodCallCodeFragments);

methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(foreignKey, annotations, GenerateFluentApi));
methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(navigation, annotations, GenerateFluentApi));

return methodCallCodeFragments;
}

/// <inheritdoc />
public virtual IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
INavigation navigation, IDictionary<string, IAnnotation> annotations)
{
var methodCallCodeFragments = new List<MethodCallCodeFragment>();

methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(navigation, annotations, GenerateFluentApi));

return methodCallCodeFragments;
}

/// <inheritdoc />
public virtual IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
ISkipNavigation navigation, IDictionary<string, IAnnotation> annotations)
{
var methodCallCodeFragments = new List<MethodCallCodeFragment>();

methodCallCodeFragments.AddRange(GenerateFluentApiCallsHelper(navigation, annotations, GenerateFluentApi));

return methodCallCodeFragments;
}
Expand Down Expand Up @@ -501,6 +523,46 @@ protected virtual MethodCallCodeFragment GenerateFluentApi([NotNull] IForeignKey
return null;
}

/// <summary>
/// <para>
/// Returns a fluent API call for the given <paramref name="annotation" />, or <see langword="null" />
/// if no fluent API call exists for it.
/// </para>
/// <para>
/// The default implementation always returns <see langword="null" />.
/// </para>
/// </summary>
/// <param name="navigation"> The <see cref="INavigation" />. </param>
/// <param name="annotation"> The <see cref="IAnnotation" />. </param>
/// <returns> <see langword="null" />. </returns>
protected virtual MethodCallCodeFragment GenerateFluentApi([NotNull] INavigation navigation, [NotNull] IAnnotation annotation)
{
Check.NotNull(navigation, nameof(navigation));
Check.NotNull(annotation, nameof(annotation));

return null;
}

/// <summary>
/// <para>
/// Returns a fluent API call for the given <paramref name="annotation" />, or <see langword="null" />
/// if no fluent API call exists for it.
/// </para>
/// <para>
/// The default implementation always returns <see langword="null" />.
/// </para>
/// </summary>
/// <param name="navigation"> The <see cref="ISkipNavigation" />. </param>
/// <param name="annotation"> The <see cref="IAnnotation" />. </param>
/// <returns> <see langword="null" />. </returns>
protected virtual MethodCallCodeFragment GenerateFluentApi([NotNull] ISkipNavigation navigation, [NotNull] IAnnotation annotation)
{
Check.NotNull(navigation, nameof(navigation));
Check.NotNull(annotation, nameof(annotation));

return null;
}

/// <summary>
/// <para>
/// Returns a fluent API call for the given <paramref name="annotation" />, or <see langword="null" />
Expand Down
20 changes: 20 additions & 0 deletions src/EFCore.Relational/Design/IAnnotationCodeGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -121,6 +121,26 @@ IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
[NotNull] IForeignKey foreignKey, [NotNull] IDictionary<string, IAnnotation> annotations)
=> Array.Empty<MethodCallCodeFragment>();

/// <summary>
/// For the given annotations which have corresponding fluent API calls, returns those fluent API calls
/// and removes the annotations.
/// </summary>
/// <param name="navigation"> The navigation to which the annotations are applied. </param>
/// <param name="annotations"> The set of annotations from which to generate fluent API calls. </param>
IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
[NotNull] INavigation navigation, [NotNull] IDictionary<string, IAnnotation> annotations)
=> Array.Empty<MethodCallCodeFragment>();

/// <summary>
/// For the given annotations which have corresponding fluent API calls, returns those fluent API calls
/// and removes the annotations.
/// </summary>
/// <param name="navigation"> The skip navigation to which the annotations are applied. </param>
/// <param name="annotations"> The set of annotations from which to generate fluent API calls. </param>
IReadOnlyList<MethodCallCodeFragment> GenerateFluentApiCalls(
[NotNull] ISkipNavigation navigation, [NotNull] IDictionary<string, IAnnotation> annotations)
=> Array.Empty<MethodCallCodeFragment>();

/// <summary>
/// For the given annotations which have corresponding fluent API calls, returns those fluent API calls
/// and removes the annotations.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public static string GetDefaultName(
{
IForeignKey linkedForeignKey = null;
foreach (var otherForeignKey in rootForeignKey.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys()))
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys()))
{
if (principalStoreObject.Name == otherForeignKey.PrincipalEntityType.GetTableName()
&& principalStoreObject.Schema == otherForeignKey.PrincipalEntityType.GetSchema()
Expand Down Expand Up @@ -196,7 +196,7 @@ public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeign
// Using a hashset is detrimental to the perf when there are no cycles
for (var i = 0; i < Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable; i++)
{
IForeignKey linkedKey = null;
IForeignKey linkedForeignKey = null;
foreach (var otherForeignKey in rootForeignKey.DeclaringEntityType
.FindRowInternalForeignKeys(storeObject)
.SelectMany(fk => fk.PrincipalEntityType.GetForeignKeys()))
Expand All @@ -207,17 +207,17 @@ public static IForeignKey FindSharedObjectRootForeignKey([NotNull] this IForeign
otherForeignKey.PrincipalEntityType.GetSchema()))
== foreignKeyName)
{
linkedKey = otherForeignKey;
linkedForeignKey = otherForeignKey;
break;
}
}

if (linkedKey == null)
if (linkedForeignKey == null)
{
break;
}

rootForeignKey = linkedKey;
rootForeignKey = linkedForeignKey;
}

return rootForeignKey == foreignKey ? null : rootForeignKey;
Expand Down
20 changes: 19 additions & 1 deletion src/EFCore.Relational/Extensions/RelationalPropertyExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -825,7 +825,25 @@ public static bool IsColumnNullable([NotNull] this IProperty property, in StoreO

return property.IsNullable
|| (property.DeclaringEntityType.BaseType != null && property.DeclaringEntityType.GetDiscriminatorProperty() != null)
|| property.DeclaringEntityType.FindRowInternalForeignKeys(storeObject).Any();
|| IsOptionalSharingDependent(property.DeclaringEntityType, storeObject, 0);
}

private static bool IsOptionalSharingDependent(IEntityType entityType, in StoreObjectIdentifier storeObject, int recursionDepth)
{
if (recursionDepth++ == Metadata.Internal.RelationalEntityTypeExtensions.MaxEntityTypesSharingTable)
{
return true;
}

bool? optional = null;
foreach (var linkingForeignKey in entityType.FindRowInternalForeignKeys(storeObject))
{
optional = (optional ?? true)
&& (!linkingForeignKey.IsRequiredDependent
|| IsOptionalSharingDependent(linkingForeignKey.PrincipalEntityType, storeObject, recursionDepth));
}

return optional ?? false;
}

/// <summary>
Expand Down
2 changes: 1 addition & 1 deletion src/EFCore.Relational/Storage/ParameterNameGenerator.cs
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public class ParameterNameGenerator
/// Generates the next unique parameter name.
/// </summary>
/// <returns> The generated name. </returns>
public virtual string GenerateNext() => string.Format(CultureInfo.InvariantCulture, "p{0}", _count++);
public virtual string GenerateNext() => "p" + _count++;

/// <summary>
/// Resets the generator, meaning it can reuse previously generated names.
Expand Down
Loading