From 8fdea84ee29789c8a969c43fa844ab4eb19e3c5a Mon Sep 17 00:00:00 2001 From: Neil Bostrom Date: Fri, 10 May 2019 22:51:54 +0100 Subject: [PATCH] Moved CheckConstraints from IModel to IEntityType to store annotations on each entity --- .../Design/CSharpSnapshotGenerator.cs | 54 ++++- .../RelationalEntityTypeBuilderExtensions.cs | 31 +-- .../RelationalEntityTypeExtensions.cs | 111 ++++++++++ .../Extensions/RelationalModelExtensions.cs | 126 ----------- .../Metadata/ICheckConstraint.cs | 16 +- .../Metadata/IConventionCheckConstraint.cs | 6 +- .../Metadata/Internal/CheckConstraint.cs | 206 ++++-------------- .../Metadata/Internal/TableMapping.cs | 10 + .../Internal/MigrationsModelDiffer.cs | 25 ++- .../Properties/RelationalStrings.Designer.cs | 6 + .../Properties/RelationalStrings.resx | 3 + .../Migrations/ModelSnapshotSqlServerTest.cs | 50 ++++- .../RelationalBuilderExtensionsTest.cs | 57 ++++- ...RelationalMetadataBuilderExtensionsTest.cs | 14 +- 14 files changed, 355 insertions(+), 360 deletions(-) diff --git a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs index c96febf3492..ec3e743f4fe 100644 --- a/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs +++ b/src/EFCore.Design/Migrations/Design/CSharpSnapshotGenerator.cs @@ -199,6 +199,8 @@ protected virtual void GenerateEntityType( GenerateEntityTypeAnnotations(builderName, entityType, stringBuilder); + GenerateCheckConstraints(builderName, entityType, stringBuilder); + if (ownerNavigation != null) { GenerateRelationships(builderName, entityType, stringBuilder); @@ -767,7 +769,8 @@ protected virtual void GenerateEntityTypeAnnotations( CoreAnnotationNames.ChangeTrackingStrategy, CoreAnnotationNames.ConstructorBinding, CoreAnnotationNames.DefiningQuery, - CoreAnnotationNames.QueryFilter); + CoreAnnotationNames.QueryFilter, + RelationalAnnotationNames.CheckConstraints); if (annotations.Count > 0) { @@ -790,6 +793,55 @@ protected virtual void GenerateEntityTypeAnnotations( } } + /// + /// Generates code for objects. + /// + /// The name of the builder variable. + /// The entity type. + /// The builder code is added to. + protected virtual void GenerateCheckConstraints( + [NotNull] string builderName, + [NotNull] IEntityType entityType, + [NotNull] IndentedStringBuilder stringBuilder) + { + Check.NotNull(builderName, nameof(builderName)); + Check.NotNull(entityType, nameof(entityType)); + Check.NotNull(stringBuilder, nameof(stringBuilder)); + + var constraintsForEntity = entityType.GetCheckConstraints(); + + foreach (var checkConstraint in constraintsForEntity) + { + stringBuilder.AppendLine(); + + GenerateCheckConstraint(builderName, checkConstraint, stringBuilder); + } + } + + /// + /// Generates code for an . + /// + /// The name of the builder variable. + /// The check constraint. + /// The builder code is added to. + protected virtual void GenerateCheckConstraint( + [NotNull] string builderName, + [NotNull] ICheckConstraint checkConstraint, + [NotNull] IndentedStringBuilder stringBuilder) + { + Check.NotNull(builderName, nameof(builderName)); + Check.NotNull(checkConstraint, nameof(checkConstraint)); + Check.NotNull(stringBuilder, nameof(stringBuilder)); + + stringBuilder + .Append(builderName) + .Append(".HasCheckConstraint(") + .Append(Code.Literal(checkConstraint.Name)) + .Append(", ") + .Append(Code.Literal(checkConstraint.Sql)) + .AppendLine(");"); + } + /// /// Generates code for objects. /// diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs index 1f91412dcea..dc1733f6114 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeBuilderExtensions.cs @@ -353,24 +353,20 @@ public static EntityTypeBuilder HasCheckConstraint( var entityType = entityTypeBuilder.Metadata; - var tableName = entityType.GetTableName(); - var schema = entityType.GetSchema(); - - var constraint = entityType.Model.FindCheckConstraint(name, tableName, schema); + var constraint = entityType.FindCheckConstraint(name); if (constraint != null) { if (constraint.Sql == sql) { - ((CheckConstraint)constraint).UpdateConfigurationSource(ConfigurationSource.Explicit); return entityTypeBuilder; } - entityType.Model.RemoveCheckConstraint(name, tableName, schema); + entityType.RemoveCheckConstraint(name); } if (sql != null) { - entityType.Model.AddCheckConstraint(sql, name, tableName, schema); + entityType.AddCheckConstraint(name, sql); } return entityTypeBuilder; @@ -414,15 +410,12 @@ public static IConventionEntityTypeBuilder HasCheckConstraint( var entityType = entityTypeBuilder.Metadata; - var tableName = entityType.GetTableName(); - var schema = entityType.GetSchema(); - - var constraint = entityType.Model.FindCheckConstraint(name, tableName, schema); + var constraint = entityType.FindCheckConstraint(name); if (constraint != null) { if (constraint.Sql == sql) { - ((CheckConstraint)constraint).UpdateConfigurationSource( + ((CheckConstraint)constraint).UpdateConfigurationSource( fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); return entityTypeBuilder; } @@ -433,19 +426,19 @@ public static IConventionEntityTypeBuilder HasCheckConstraint( return null; } - entityType.Model.RemoveCheckConstraint(name, tableName, schema); + entityType.RemoveCheckConstraint(name); } if (sql != null) { - entityType.Model.AddCheckConstraint(sql, name, tableName, schema, fromDataAnnotation); + entityType.AddCheckConstraint(name, sql, fromDataAnnotation); } return entityTypeBuilder; } /// - /// Returns a value indicating whether the discriminator column can be configured. + /// Returns a value indicating whether the check constraint can be configured. /// /// The builder for the entity type being configured. /// The name of the check constraint. @@ -462,16 +455,12 @@ public static bool CanSetCheckConstraint( Check.NotEmpty(name, nameof(name)); Check.NullButNotEmpty(sql, nameof(sql)); - var entityType = entityTypeBuilder.Metadata; - var tableName = entityType.GetTableName(); - var schema = entityType.GetSchema(); - - var constraint = entityType.Model.FindCheckConstraint(name, tableName, schema); + var constraint = entityTypeBuilder.Metadata.FindCheckConstraint(name); return constraint == null || constraint.Sql == sql || (fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention) - .Overrides(entityTypeBuilder.Metadata.GetDiscriminatorPropertyConfigurationSource()); + .Overrides(constraint.GetConfigurationSource()); } } } diff --git a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs index 53880f6d37c..2f5990600d5 100644 --- a/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalEntityTypeExtensions.cs @@ -2,11 +2,13 @@ // Licensed under the Apache License, Version 2.0. See License.txt in the project root for license information. using System; +using System.Collections.Generic; using System.Reflection; using JetBrains.Annotations; using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Infrastructure; using Microsoft.EntityFrameworkCore.Metadata; +using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.Utilities; // ReSharper disable once CheckNamespace @@ -142,5 +144,114 @@ public static void SetSchema( public static ConfigurationSource? GetSchemaConfigurationSource([NotNull] this IConventionEntityType entityType) => entityType.FindAnnotation(RelationalAnnotationNames.Schema) ?.GetConfigurationSource(); + + /// + /// Finds an with the given name. + /// + /// The entity type to find the check constraint for. + /// The check constraint name. + /// + /// The or null if no check constraint with the + /// given name in the given entity type was found. + /// + public static ICheckConstraint FindCheckConstraint( + [NotNull] this IEntityType entityType, [NotNull] string name) + { + Check.NotEmpty(name, nameof(name)); + + return CheckConstraint.FindCheckConstraint(entityType, name); + } + + /// + /// Finds an with the given name. + /// + /// The entity type to find the check constraint for. + /// The check constraint name. + /// + /// The or null if no check constraint with the + /// given name in the given entity type was found. + /// + public static IConventionCheckConstraint FindCheckConstraint( + [NotNull] this IConventionEntityType entityType, [NotNull] string name) + => (IConventionCheckConstraint)((IEntityType)entityType).FindCheckConstraint(name); + + /// + /// Creates a new check constraint with the given name on entity type. Throws an exception + /// if a check constraint with the same name exists on the same entity type. + /// + /// The entity type to add the check constraint to. + /// The check constraint name. + /// The logical constraint sql used in the check constraint. + /// The new check constraint. + public static ICheckConstraint AddCheckConstraint( + [NotNull] this IMutableEntityType entityType, + [NotNull] string name, + [NotNull] string sql) + { + Check.NotEmpty(name, nameof(name)); + Check.NotEmpty(sql, nameof(sql)); + + return new CheckConstraint(entityType, name, sql, ConfigurationSource.Explicit); + } + + /// + /// Creates a new check constraint with the given name on entity type. Throws an exception + /// if a check constraint with the same name exists on the same entity type. + /// + /// The entity type to add the check constraint to. + /// The check constraint name. + /// The logical constraint sql used in the check constraint. + /// Indicates whether the configuration was specified using a data annotation. + /// The new check constraint. + public static IConventionCheckConstraint AddCheckConstraint( + [NotNull] this IConventionEntityType entityType, + [NotNull] string name, + [NotNull] string sql, + bool fromDataAnnotation = false) + { + Check.NotEmpty(name, nameof(name)); + Check.NotEmpty(sql, nameof(sql)); + + return new CheckConstraint( + (IMutableEntityType)entityType, name, sql, + fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); + } + + /// + /// Removes the with the given name. + /// + /// The entity type to remove the check constraint from. + /// The check constraint name to be removed. + /// + /// True if the is successfully found and removed; otherwise, false. + /// + public static bool RemoveCheckConstraint( + [NotNull] this IMutableEntityType entityType, + [NotNull] string name) + { + Check.NotEmpty(name, nameof(name)); + + return CheckConstraint.RemoveCheckConstraint(entityType, name); + } + + /// + /// Removes the with the given name. + /// + /// The entity type to remove the check constraint from. + /// The check constraint name. + /// + /// True if the is successfully found and removed; otherwise, false. + /// + public static bool RemoveCheckConstraint( + [NotNull] this IConventionEntityType entityType, + [NotNull] string name) + => RemoveCheckConstraint((IMutableEntityType)entityType, name); + + /// + /// Returns all contained in the entity type. + /// + /// The entity type to get the check constraints for. + public static IEnumerable GetCheckConstraints([NotNull] this IEntityType entityType) + => CheckConstraint.GetCheckConstraints(entityType); } } diff --git a/src/EFCore.Relational/Extensions/RelationalModelExtensions.cs b/src/EFCore.Relational/Extensions/RelationalModelExtensions.cs index 821607aba34..fd26fb76755 100644 --- a/src/EFCore.Relational/Extensions/RelationalModelExtensions.cs +++ b/src/EFCore.Relational/Extensions/RelationalModelExtensions.cs @@ -210,132 +210,6 @@ public static IReadOnlyList GetSequences([NotNull] this IMutab public static IReadOnlyList GetSequences([NotNull] this IConventionModel model) => (IReadOnlyList)((IModel)model).GetSequences(); - /// - /// Finds an with the given name. - /// - /// The model to find the check constraint in. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// - /// The or null if no check constraint with the given name in - /// the given schema was found. - /// - public static ICheckConstraint FindCheckConstraint( - [NotNull] this IModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema = null) - { - Check.NotEmpty(name, nameof(name)); - Check.NotEmpty(table, nameof(table)); - Check.NullButNotEmpty(schema, nameof(schema)); - - return CheckConstraint.FindCheckConstraint(model, name, table, schema); - } - - /// - /// Finds an with the given name. - /// - /// The model to find the check constraint in. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// - /// The or null if no check constraint with the given name in - /// the given schema was found. - /// - public static IConventionCheckConstraint FindCheckConstraint( - [NotNull] this IConventionModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema = null) - => (IConventionCheckConstraint)((IModel)model).FindCheckConstraint(name, table, schema); - - /// - /// Either returns the existing with the given name in the given schema - /// or creates a new check constraint with the given name and schema. - /// - /// The model to add the check constraint to. - /// The logical constraint sql used in the check constraint. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// The check constraint. - public static ICheckConstraint AddCheckConstraint( - [NotNull] this IMutableModel model, - [NotNull] string sql, - [NotNull] string name, - [NotNull] string table, - [CanBeNull] string schema = null) - => new CheckConstraint(model, name, sql, table, schema, ConfigurationSource.Explicit); - - /// - /// Either returns the existing with the given name in the given schema - /// or creates a new check constraint with the given name and schema. - /// - /// The model to add the check constraint to. - /// The logical constraint sql used in the check constraint. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// Indicates whether the configuration was specified using a data annotation. - /// The check constraint. - public static IConventionCheckConstraint AddCheckConstraint( - [NotNull] this IConventionModel model, - [NotNull] string sql, - [NotNull] string name, - [NotNull] string table, - [CanBeNull] string schema = null, - bool fromDataAnnotation = false) - => new CheckConstraint( - (IMutableModel)model, name, sql, table, schema, - fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention); - - /// - /// Removes the with the given name. - /// - /// The model to remove the check constraint from. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// - /// The removed or null if no check constraint with the given name in - /// the given schema was found. - /// - public static ICheckConstraint RemoveCheckConstraint( - [NotNull] this IMutableModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema = null) - { - Check.NotEmpty(name, nameof(name)); - Check.NotEmpty(table, nameof(table)); - Check.NullButNotEmpty(schema, nameof(schema)); - - return CheckConstraint.RemoveCheckConstraint(model, name, table, schema); - } - - /// - /// Removes the with the given name. - /// - /// The model to remove the check constraint from. - /// The check constraint name. - /// The table that contains the check constraint. - /// The table schema that contains the check constraint. - /// - /// The removed or null if no check constraint with the given name in - /// the given schema was found. - /// - public static IConventionCheckConstraint RemoveCheckConstraint( - [NotNull] this IConventionModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema = null) - => (IConventionCheckConstraint)((IMutableModel)model).RemoveCheckConstraint(name, table, schema); - - /// - /// Returns all s contained in the model. - /// - /// The model to get the check constraints in. - public static IReadOnlyList GetCheckConstraints([NotNull] this IModel model) - => CheckConstraint.GetCheckConstraints(model); - - /// - /// Returns all s contained in the model. - /// - /// The model to get the check constraints in. - public static IReadOnlyList GetCheckConstraints([NotNull] this IConventionModel model) - => (IReadOnlyList)((IModel)model).GetCheckConstraints(); - /// /// Finds a that is mapped to the method represented by the given . /// diff --git a/src/EFCore.Relational/Metadata/ICheckConstraint.cs b/src/EFCore.Relational/Metadata/ICheckConstraint.cs index ba87f5e2afd..3a29c46c96f 100644 --- a/src/EFCore.Relational/Metadata/ICheckConstraint.cs +++ b/src/EFCore.Relational/Metadata/ICheckConstraint.cs @@ -4,7 +4,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata { /// - /// Represents a check constraint in the . + /// Represents a check constraint in the . /// public interface ICheckConstraint { @@ -14,19 +14,9 @@ public interface ICheckConstraint string Name { get; } /// - /// The database table that contains the check constraint. + /// The in which this check constraint is defined. /// - string Table { get; } - - /// - /// The database table schema that contains the check constraint. - /// - string Schema { get; } - - /// - /// The in which this check constraint is defined. - /// - IModel Model { get; } + IEntityType EntityType { get; } /// /// Gets the constraint sql used in a check constraint in the database. diff --git a/src/EFCore.Relational/Metadata/IConventionCheckConstraint.cs b/src/EFCore.Relational/Metadata/IConventionCheckConstraint.cs index 7766d3a020a..64bbeb0eef1 100644 --- a/src/EFCore.Relational/Metadata/IConventionCheckConstraint.cs +++ b/src/EFCore.Relational/Metadata/IConventionCheckConstraint.cs @@ -4,14 +4,14 @@ namespace Microsoft.EntityFrameworkCore.Metadata { /// - /// Represents a check constraint in the . + /// Represents a check constraint in the . /// public interface IConventionCheckConstraint : ICheckConstraint { /// - /// The in which this check constraint is defined. + /// The in which this check constraint is defined. /// - new IConventionModel Model { get; } + new IConventionEntityType EntityType { get; } /// /// Returns the configuration source for this . diff --git a/src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs b/src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs index 092bbb64daa..63d2fce7615 100644 --- a/src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs +++ b/src/EFCore.Relational/Metadata/Internal/CheckConstraint.cs @@ -19,7 +19,7 @@ namespace Microsoft.EntityFrameworkCore.Metadata.Internal /// public class CheckConstraint : IConventionCheckConstraint { - private readonly string _keyName; + private ConfigurationSource _configurationSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -28,44 +28,35 @@ public class CheckConstraint : IConventionCheckConstraint /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public CheckConstraint( - [NotNull] IMutableModel model, + [NotNull] IMutableEntityType entityType, [NotNull] string name, [NotNull] string sql, - [NotNull] string table, - [CanBeNull] string schema, ConfigurationSource configurationSource) - : this(model, GetKeyName(name, table, schema)) { + Check.NotNull(entityType, nameof(entityType)); Check.NotEmpty(name, nameof(name)); Check.NotEmpty(sql, nameof(sql)); - Check.NotEmpty(table, nameof(table)); - Check.NullButNotEmpty(schema, nameof(schema)); - var dataDictionary = GetAnnotationsDictionary(model); + EntityType = entityType; + Name = name; + Sql = sql; + _configurationSource = configurationSource; + + var dataDictionary = GetAnnotationsDictionary(EntityType); if (dataDictionary == null) { - dataDictionary = new Dictionary(); - model[RelationalAnnotationNames.CheckConstraints] = dataDictionary; + dataDictionary = new Dictionary(); + ((IMutableEntityType)EntityType).SetOrRemoveAnnotation(RelationalAnnotationNames.CheckConstraints, dataDictionary); } - var data = new CheckConstraintData + if (dataDictionary.ContainsKey(Name)) { - Name = name, - Sql = sql, - Table = table, - Schema = schema, - ConfigurationSource = configurationSource - }.Serialize(); - - dataDictionary.Add(_keyName, data); - } - - private CheckConstraint([NotNull] IModel model, [NotNull] string keyName) - { - Check.NotNull(model, nameof(model)); - - Model = model; - _keyName = keyName; + throw new InvalidOperationException(RelationalStrings.DuplicateCheckConstraint(Name, EntityType.DisplayName())); + } + else + { + dataDictionary.Add(name, this); + } } /// @@ -74,12 +65,11 @@ private CheckConstraint([NotNull] IModel model, [NotNull] string keyName) /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public static IReadOnlyList GetCheckConstraints([NotNull] IModel model) + public static IEnumerable GetCheckConstraints([NotNull] IEntityType entityType) { - Check.NotNull(model, nameof(model)); + Check.NotNull(entityType, nameof(entityType)); - return GetAnnotationsDictionary(model)? - .Select(a => new CheckConstraint(model, a.Key)).ToList() ?? new List(); + return GetAnnotationsDictionary(entityType)?.Values ?? Enumerable.Empty(); } /// @@ -89,12 +79,16 @@ public static IReadOnlyList GetCheckConstraints([NotNull] IMode /// doing so can result in application failures when updating to a new Entity Framework Core release. /// public static ICheckConstraint FindCheckConstraint( - [NotNull] IModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema) + [NotNull] IEntityType entityType, [NotNull] string name) { - var dataDictionary = GetAnnotationsDictionary(model); - var annotationKey = GetKeyName(name, table, schema); + var dataDictionary = GetAnnotationsDictionary(entityType); + + if (dataDictionary == null) + { + return null; + } - return dataDictionary?.ContainsKey(annotationKey) == true ? new CheckConstraint(model, annotationKey) : null; + return dataDictionary.TryGetValue(name, out var checkConstraint) ? checkConstraint : null; } /// @@ -103,41 +97,21 @@ public static ICheckConstraint FindCheckConstraint( /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public static ICheckConstraint RemoveCheckConstraint( - [NotNull] IMutableModel model, [NotNull] string name, [NotNull] string table, [CanBeNull] string schema) + public static bool RemoveCheckConstraint( + [NotNull] IMutableEntityType entityType, [NotNull] string name) { - var dataDictionary = GetAnnotationsDictionary(model); - var annotationKey = GetKeyName(name, table, schema); + var dataDictionary = GetAnnotationsDictionary(entityType); - return dataDictionary.Remove(annotationKey) ? new CheckConstraint(model, annotationKey) : null; + return dataDictionary?.Remove(name) ?? false; } - private static string GetKeyName(string name, string table, string schema = null) - => $"{schema}.{table}:{name}"; - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual IModel Model { get; } - - /// - /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to - /// the same compatibility standards as public APIs. It may be changed or removed without notice in - /// any release. You should only use it directly in your code with extreme caution and knowing that - /// doing so can result in application failures when updating to a new Entity Framework Core release. - /// - public virtual string Name => GetData().Name; - /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual string Table => GetData().Table; + public virtual IEntityType EntityType { get; } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -145,7 +119,7 @@ private static string GetKeyName(string name, string table, string schema = null /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual string Schema => GetData().Schema ?? Model.GetDefaultSchema(); + public virtual string Name { get; } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -153,7 +127,7 @@ private static string GetKeyName(string name, string table, string schema = null /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual string Sql => GetData().Sql; + public virtual string Sql { get; } /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -161,7 +135,7 @@ private static string GetKeyName(string name, string table, string schema = null /// any release. You should only use it directly in your code with extreme caution and knowing that /// doing so can result in application failures when updating to a new Entity Framework Core release. /// - public virtual ConfigurationSource GetConfigurationSource() => GetData().ConfigurationSource; + public virtual ConfigurationSource GetConfigurationSource() => _configurationSource; /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to @@ -171,113 +145,15 @@ private static string GetKeyName(string name, string table, string schema = null /// public virtual void UpdateConfigurationSource(ConfigurationSource configurationSource) { - var data = GetData(); - data.ConfigurationSource = configurationSource.Max(data.ConfigurationSource); - SetData(data); + _configurationSource = configurationSource.Max(_configurationSource); } - private CheckConstraintData GetData() => CheckConstraintData.Deserialize(GetAnnotationsDictionary(Model)?[_keyName]); - - private void SetData(CheckConstraintData data) + private static Dictionary GetAnnotationsDictionary(IEntityType entityType) { - var dataDictionary = GetAnnotationsDictionary(Model); - if (dataDictionary == null) - { - dataDictionary = new Dictionary(); - ((IMutableModel)Model)[RelationalAnnotationNames.CheckConstraints] = dataDictionary; - } - - dataDictionary[_keyName] = data.Serialize(); + return (Dictionary)entityType[RelationalAnnotationNames.CheckConstraints]; } - private static Dictionary GetAnnotationsDictionary(IModel model) => - (Dictionary)model[RelationalAnnotationNames.CheckConstraints]; - /// - IConventionModel IConventionCheckConstraint.Model => (IConventionModel)Model; - - private class CheckConstraintData - { - public string Name { get; set; } - - public string Table { get; set; } - - public string Schema { get; set; } - - public string Sql { get; set; } - - public ConfigurationSource ConfigurationSource { get; set; } - - public string Serialize() - { - var builder = new StringBuilder(); - - EscapeAndQuote(builder, Name); - builder.Append(", "); - EscapeAndQuote(builder, Table); - builder.Append(", "); - EscapeAndQuote(builder, Schema); - builder.Append(", "); - EscapeAndQuote(builder, Sql); - builder.Append(", "); - EscapeAndQuote(builder, ConfigurationSource); - - return builder.ToString(); - } - - public static CheckConstraintData Deserialize(string value) - { - try - { - var data = new CheckConstraintData(); - - // ReSharper disable PossibleInvalidOperationException - var position = 0; - data.Name = ExtractValue(value, ref position); - data.Table = ExtractValue(value, ref position); - data.Schema = ExtractValue(value, ref position); - data.Sql = ExtractValue(value, ref position); - data.ConfigurationSource = - (ConfigurationSource)Enum.Parse(typeof(ConfigurationSource), ExtractValue(value, ref position)); - // ReSharper restore PossibleInvalidOperationException - - return data; - } - catch (Exception ex) - { - throw new ArgumentException(RelationalStrings.BadCheckConstraintString, ex); - } - } - - private static string ExtractValue(string value, ref int position) - { - position = value.IndexOf('\'', position) + 1; - - var end = value.IndexOf('\'', position); - - while (end + 1 < value.Length - && value[end + 1] == '\'') - { - end = value.IndexOf('\'', end + 2); - } - - var extracted = value.Substring(position, end - position).Replace("''", "'"); - position = end + 1; - - return extracted.Length == 0 ? null : extracted; - } - - private static void EscapeAndQuote(StringBuilder builder, object value) - { - builder.Append("'"); - - if (value != null) - { - builder.Append(value.ToString().Replace("'", "''")); - } - - builder.Append("'"); - } - } + IConventionEntityType IConventionCheckConstraint.EntityType => (IConventionEntityType)EntityType; } } diff --git a/src/EFCore.Relational/Metadata/Internal/TableMapping.cs b/src/EFCore.Relational/Metadata/Internal/TableMapping.cs index b570c93bb00..b66a8ed8b29 100644 --- a/src/EFCore.Relational/Metadata/Internal/TableMapping.cs +++ b/src/EFCore.Relational/Metadata/Internal/TableMapping.cs @@ -136,6 +136,16 @@ public virtual IEnumerable GetForeignKeys() && fk.Properties.Select(p => p.GetColumnName()) .SequenceEqual(fk.PrincipalKey.Properties.Select(p => p.GetColumnName())))); + /// + /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to + /// the same compatibility standards as public APIs. It may be changed or removed without notice in + /// any release. You should only use it directly in your code with extreme caution and knowing that + /// doing so can result in application failures when updating to a new Entity Framework Core release. + /// + public virtual IEnumerable GetCheckConstraints() + => EntityTypes.SelectMany(e => CheckConstraint.GetCheckConstraints(e)) + .Distinct((x, y) => x.Name == y.Name); + /// /// This is an internal API that supports the Entity Framework Core infrastructure and not subject to /// the same compatibility standards as public APIs. It may be changed or removed without notice in diff --git a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs index 2266d5ed8ae..05db036be92 100644 --- a/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs +++ b/src/EFCore.Relational/Migrations/Internal/MigrationsModelDiffer.cs @@ -364,7 +364,6 @@ protected virtual IEnumerable Diff( .Concat(Diff(GetSchemas(source), GetSchemas(target), diffContext)) .Concat(Diff(diffContext.GetSourceTables(), diffContext.GetTargetTables(), diffContext)) .Concat(Diff(source.GetSequences(), target.GetSequences(), diffContext)) - .Concat(Diff(source.GetCheckConstraints(), target.GetCheckConstraints(), diffContext)) .Concat( Diff( diffContext.GetSourceTables().SelectMany(s => s.GetForeignKeys()), @@ -431,7 +430,6 @@ protected virtual IEnumerable Add([NotNull] IModel target, [ .Concat(GetSchemas(target).SelectMany(t => Add(t, diffContext))) .Concat(diffContext.GetTargetTables().SelectMany(t => Add(t, diffContext))) .Concat(target.GetSequences().SelectMany(t => Add(t, diffContext))) - .Concat(target.GetCheckConstraints().SelectMany(t => Add(t, diffContext))) .Concat(diffContext.GetTargetTables().SelectMany(t => t.GetForeignKeys()).SelectMany(k => Add(k, diffContext))); /// @@ -443,8 +441,7 @@ protected virtual IEnumerable Add([NotNull] IModel target, [ protected virtual IEnumerable Remove([NotNull] IModel source, [NotNull] DiffContext diffContext) => DiffAnnotations(source, null) .Concat(diffContext.GetSourceTables().SelectMany(t => Remove(t, diffContext))) - .Concat(source.GetSequences().SelectMany(t => Remove(t, diffContext))) - .Concat(source.GetCheckConstraints().SelectMany(t => Remove(t, diffContext))); + .Concat(source.GetSequences().SelectMany(t => Remove(t, diffContext))); #endregion @@ -563,7 +560,8 @@ protected virtual IEnumerable Diff( var operations = DiffAnnotations(source, target) .Concat(Diff(source.GetProperties(), target.GetProperties(), diffContext)) .Concat(Diff(source.GetKeys(), target.GetKeys(), diffContext)) - .Concat(Diff(source.GetIndexes(), target.GetIndexes(), diffContext)); + .Concat(Diff(source.GetIndexes(), target.GetIndexes(), diffContext)) + .Concat(Diff(source.GetCheckConstraints(), target.GetCheckConstraints(), diffContext)); foreach (var operation in operations) { yield return operation; @@ -1383,9 +1381,8 @@ protected virtual IEnumerable Diff( Diff, Add, Remove, - (s, t, c) => string.Equals(s.Schema, t.Schema, StringComparison.OrdinalIgnoreCase) + (s, t, c) => c.FindSourceTable(s.EntityType) == c.FindSource(c.FindTargetTable(t.EntityType)) && string.Equals(s.Name, t.Name, StringComparison.OrdinalIgnoreCase) - && string.Equals(s.Table, t.Table, StringComparison.OrdinalIgnoreCase) && string.Equals(s.Sql, t.Sql, StringComparison.OrdinalIgnoreCase)); /// @@ -1402,12 +1399,14 @@ protected virtual IEnumerable Diff( /// protected virtual IEnumerable Add([NotNull] ICheckConstraint target, [NotNull] DiffContext diffContext) { + var targetEntityType = target.EntityType.RootType(); + var operation = new CreateCheckConstraintOperation { - Schema = target.Schema, Name = target.Name, - Table = target.Table, - Sql = target.Sql + Sql = target.Sql, + Schema = targetEntityType.GetSchema(), + Table = targetEntityType.GetTableName() }; operation.Sql = target.Sql; @@ -1422,11 +1421,13 @@ protected virtual IEnumerable Add([NotNull] ICheckConstraint /// protected virtual IEnumerable Remove([NotNull] ICheckConstraint source, [NotNull] DiffContext diffContext) { + var sourceEntityType = source.EntityType.RootType(); + var operation = new DropCheckConstraintOperation { - Schema = source.Schema, Name = source.Name, - Table = source.Table + Schema = sourceEntityType.GetSchema(), + Table = sourceEntityType.GetTableName() }; operation.AddAnnotations(MigrationsAnnotations.ForRemove(source)); diff --git a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs index c9740772b0d..aa105752edd 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs +++ b/src/EFCore.Relational/Properties/RelationalStrings.Designer.cs @@ -321,6 +321,12 @@ public static string ConflictingColumnServerGeneration([CanBeNull] object confli GetString("ConflictingColumnServerGeneration", nameof(conflictingConfiguration), nameof(property), nameof(existingConfiguration)), conflictingConfiguration, property, existingConfiguration); + /// + /// The check constraint '{checkConstraint}' cannot be added to the entity type '{entityType}' because another check constraint with the same name already exists. + /// + public static string DuplicateCheckConstraint([CanBeNull] object checkConstraint, [CanBeNull] object entityType) + => string.Format(GetString("DuplicateCheckConstraint", nameof(checkConstraint), nameof(entityType)), checkConstraint, entityType); + /// /// The foreign keys {index1} on '{entityType1}' and {index2} on '{entityType2}' are both mapped to '{table}.{foreignKeyName}' but use different columns ({columnNames1} and {columnNames2}). /// diff --git a/src/EFCore.Relational/Properties/RelationalStrings.resx b/src/EFCore.Relational/Properties/RelationalStrings.resx index cc6e575c8b2..14cfdd8979d 100644 --- a/src/EFCore.Relational/Properties/RelationalStrings.resx +++ b/src/EFCore.Relational/Properties/RelationalStrings.resx @@ -352,6 +352,9 @@ {conflictingConfiguration} cannot be set for '{property}' at the same time as {existingConfiguration}. Remove one of these values. + + The check constraint '{checkConstraint}' cannot be added to the entity type '{entityType}' because another check constraint with the same name already exists. + The foreign keys {index1} on '{entityType1}' and {index2} on '{entityType2}' are both mapped to '{table}.{foreignKeyName}' but use different columns ({columnNames1} and {columnNames2}). diff --git a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs index 3af8ed1b3b0..ce32e103df2 100644 --- a/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs +++ b/test/EFCore.Design.Tests/Migrations/ModelSnapshotSqlServerTest.cs @@ -344,8 +344,7 @@ public virtual void Sequence_is_stored_in_snapshot_as_fluent_api() }); } - //[Fact] - //Issue #15676 + [Fact] public virtual void CheckConstraint_is_stored_in_snapshot_as_fluent_api() { Test( @@ -373,7 +372,52 @@ public virtual void CheckConstraint_is_stored_in_snapshot_as_fluent_api() });"), o => { - Assert.Equal(3, o.GetAnnotations().Count()); + Assert.Equal(2, o.GetAnnotations().Count()); + }); + } + + [Fact] + public virtual void CheckConstraint_is_only_stored_in_snapshot_once_for_TPH() + { + Test( + builder => + { + builder.Entity() + .HasBaseType() + .HasCheckConstraint("CK_Customer_AlternateId", "AlternateId > Id"); + builder.Entity(); + }, + AddBoilerPlate( + GetHeading() + @" + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity"", b => + { + b.Property(""Id"") + .ValueGeneratedOnAdd() + .HasAnnotation(""SqlServer:ValueGenerationStrategy"", SqlServerValueGenerationStrategy.IdentityColumn); + + b.Property(""Discriminator"") + .IsRequired(); + + b.HasKey(""Id""); + + b.ToTable(""BaseEntity""); + + b.HasDiscriminator(""Discriminator"").HasValue(""BaseEntity""); + }); + + modelBuilder.Entity(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+DerivedEntity"", b => + { + b.HasBaseType(""Microsoft.EntityFrameworkCore.Migrations.ModelSnapshotSqlServerTest+BaseEntity""); + + b.Property(""Name""); + + b.HasDiscriminator().HasValue(""DerivedEntity""); + + b.HasCheckConstraint(""CK_Customer_AlternateId"", ""AlternateId > Id""); + });"), + o => + { + Assert.Equal(2, o.GetAnnotations().Count()); }); } diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs index 604ed744311..31bb9c96136 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalBuilderExtensionsTest.cs @@ -1,9 +1,11 @@ // 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.Collections.Generic; using System.Linq; using System.Reflection; +using Microsoft.EntityFrameworkCore.Diagnostics; using Microsoft.EntityFrameworkCore.Metadata.Builders; using Microsoft.EntityFrameworkCore.Metadata.Internal; using Microsoft.EntityFrameworkCore.TestUtilities; @@ -493,37 +495,74 @@ public void Can_set_table_and_schema_name_non_generic() public void Can_create_check_constraint() { var modelBuilder = CreateConventionModelBuilder(); + var entityType = modelBuilder.Entity().Metadata; modelBuilder .Entity() .HasCheckConstraint("CK_Customer_AlternateId", "AlternateId > Id"); - var checkConstraint = modelBuilder.Model.FindCheckConstraint("CK_Customer_AlternateId", "Customer"); + var checkConstraint = entityType.FindCheckConstraint("CK_Customer_AlternateId"); Assert.NotNull(checkConstraint); + Assert.Equal(entityType, checkConstraint.EntityType); Assert.Equal("CK_Customer_AlternateId", checkConstraint.Name); Assert.Equal("AlternateId > Id", checkConstraint.Sql); - Assert.Equal("Customer", checkConstraint.Table); - Assert.Equal(null, checkConstraint.Schema); } [Fact] - public void Can_create_check_constraint_with_schema_and_none_default_table_name() + public void Can_create_check_constraint_with_duplicate_name_replaces_existing() { var modelBuilder = CreateConventionModelBuilder(); + var entityType = modelBuilder.Entity().Metadata; modelBuilder .Entity() - .ToTable("Customizer", "db0") .HasCheckConstraint("CK_Customer_AlternateId", "AlternateId > Id"); - var checkConstraint = modelBuilder.Model.FindCheckConstraint("CK_Customer_AlternateId", "Customizer", "db0"); + modelBuilder + .Entity() + .HasCheckConstraint("CK_Customer_AlternateId", "AlternateId < Id"); + + var checkConstraint = entityType.FindCheckConstraint("CK_Customer_AlternateId"); Assert.NotNull(checkConstraint); + Assert.Equal(entityType, checkConstraint.EntityType); Assert.Equal("CK_Customer_AlternateId", checkConstraint.Name); - Assert.Equal("AlternateId > Id", checkConstraint.Sql); - Assert.Equal("Customizer", checkConstraint.Table); - Assert.Equal("db0", checkConstraint.Schema); + Assert.Equal("AlternateId < Id", checkConstraint.Sql); + } + + [Fact] + public void AddCheckConstraint_with_duplicate_names_throws_exception() + { + var entityTypeBuilder = CreateConventionModelBuilder().Entity(); + var entityType = entityTypeBuilder.Metadata; + + entityType.AddCheckConstraint("CK_Customer_AlternateId", "AlternateId > Id"); + + Assert.Equal( + RelationalStrings.DuplicateCheckConstraint("CK_Customer_AlternateId", entityType.DisplayName()), + Assert.Throws(() => + entityType.AddCheckConstraint("CK_Customer_AlternateId", "AlternateId < Id")).Message); + } + + [Fact] + public void RemoveCheckConstraint_returns_true_when_constraint_exists() + { + var entityTypeBuilder = CreateConventionModelBuilder().Entity(); + var entityType = entityTypeBuilder.Metadata; + + entityType.AddCheckConstraint("CK_Customer_AlternateId", "AlternateId > Id"); + + Assert.True(entityType.RemoveCheckConstraint("CK_Customer_AlternateId")); + } + + [Fact] + public void RemoveCheckConstraint_returns_false_when_constraint_is_missing() + { + var entityTypeBuilder = CreateConventionModelBuilder().Entity(); + var entityType = entityTypeBuilder.Metadata; + + Assert.False(entityType.RemoveCheckConstraint("CK_Customer_AlternateId")); } [Fact] diff --git a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs index f9a92a46e33..0d4acf30e17 100644 --- a/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs +++ b/test/EFCore.Relational.Tests/Metadata/RelationalMetadataBuilderExtensionsTest.cs @@ -181,19 +181,19 @@ public void Can_access_relationship() public void Can_access_check_constraint() { var typeBuilder = CreateBuilder().Entity(typeof(Splot), ConfigurationSource.Convention); - var model = typeBuilder.Metadata.Model; + var entityType = typeBuilder.Metadata; Assert.NotNull(typeBuilder.HasCheckConstraint("Splew", "s > p")); - Assert.Equal("Splew", model.GetCheckConstraints().Single().Name); - Assert.Equal("s > p", model.GetCheckConstraints().Single().Sql); + Assert.Equal("Splew", entityType.GetCheckConstraints().Single().Name); + Assert.Equal("s > p", entityType.GetCheckConstraints().Single().Sql); Assert.NotNull(typeBuilder.HasCheckConstraint("Splew", "s < p", fromDataAnnotation: true)); - Assert.Equal("Splew", model.GetCheckConstraints().Single().Name); - Assert.Equal("s < p", model.GetCheckConstraints().Single().Sql); + Assert.Equal("Splew", entityType.GetCheckConstraints().Single().Name); + Assert.Equal("s < p", entityType.GetCheckConstraints().Single().Sql); Assert.Null(typeBuilder.HasCheckConstraint("Splew", "s > p")); - Assert.Equal("Splew", model.GetCheckConstraints().Single().Name); - Assert.Equal("s < p", model.GetCheckConstraints().Single().Sql); + Assert.Equal("Splew", entityType.GetCheckConstraints().Single().Name); + Assert.Equal("s < p", entityType.GetCheckConstraints().Single().Sql); } private class Splot