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.

Sign up for GitHub

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

Jump to bottom

Make NotMapped take precedence over other attributes that reference properties #21231

Merged
merged 1 commit into from
Jun 12, 2020
Merged

Make NotMapped take precedence over other attributes that reference properties #21231

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
15 changes: 12 additions & 3 deletions src/EFCore/Metadata/Builders/IConventionEntityTypeBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -125,11 +125,15 @@ IConventionServicePropertyBuilder ServiceProperty(
[NotNull] MemberInfo memberInfo, bool fromDataAnnotation = false);

/// <summary>
/// Indicates whether the given member name is ignored for the current configuration source.
/// Indicates whether the given member name is ignored for the given configuration source.
/// </summary>
/// <param name="memberName"> The name of the member that might be ignored. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given member name is ignored. </returns>
/// <returns>
/// <see langword="false" /> if the entity type contains a member with the given name,
/// the given member name hasn't been ignored or it was ignored using a lower configuration source;
/// <see langword="true" /> otherwise.
/// </returns>
bool IsIgnored([NotNull] string memberName, bool fromDataAnnotation = false);

/// <summary>
Expand All @@ -145,11 +149,16 @@ IConventionServicePropertyBuilder ServiceProperty(
IConventionEntityTypeBuilder Ignore([NotNull] string memberName, bool fromDataAnnotation = false);

/// <summary>
/// Returns a value indicating whether the given member name can be ignored from the current configuration source.
/// Returns a value indicating whether the given member name can be ignored from the given configuration source.
/// </summary>
/// <param name="memberName"> The member name to be removed from the entity type. </param>
/// <param name="fromDataAnnotation"> Indicates whether the configuration was specified using a data annotation. </param>
/// <returns> <see langword="true" /> if the given member name can be ignored. </returns>
/// <returns>
/// <see langword="false" /> if the entity type contains a member with the given name
/// that was configured using a higher configuration source;
/// <see langword="true" /> otherwise.
/// </returns>
bool CanIgnore([NotNull] string memberName, bool fromDataAnnotation = false);

/// <summary>
Expand Down
23 changes: 12 additions & 11 deletions src/EFCore/Metadata/Conventions/IndexAttributeConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,6 @@ private void CheckIndexAttributesAndEnsureIndex(
}
else
{
// TODO See bug 21220 - we have to do this _before_ calling
// HasIndex() because during the call to HasIndex()
// IsIgnored (wrongly) returns false and we would end up
// creating a property where we shouldn't.
CheckIgnoredProperties(indexAttribute, entityType);

try
{
// Using the HasIndex(propertyNames) overload gives us
Expand All @@ -119,18 +113,25 @@ private void CheckIndexAttributesAndEnsureIndex(
: entityType.Builder.HasIndex(
indexAttribute.PropertyNames, indexAttribute.Name, fromDataAnnotation: true);
}
catch(InvalidOperationException exception)
catch (InvalidOperationException exception)
{
CheckMissingProperties(indexAttribute, entityType, exception);

throw;
}
}

if (indexBuilder != null
&& indexAttribute.GetIsUnique().HasValue)
if (indexBuilder == null)
{
CheckIgnoredProperties(indexAttribute, entityType);
}
else
{
indexBuilder.IsUnique(indexAttribute.GetIsUnique().Value, fromDataAnnotation: true);
var shouldBeUnique = indexAttribute.GetIsUnique();
if (shouldBeUnique.HasValue)
{
indexBuilder.IsUnique(shouldBeUnique.Value, fromDataAnnotation: true);
}
}
}
}
Expand All @@ -141,7 +142,7 @@ private void CheckIgnoredProperties(
{
foreach (var propertyName in indexAttribute.PropertyNames)
{
if (entityType.IsIgnored(propertyName))
if (entityType.Builder.IsIgnored(propertyName, fromDataAnnotation: true))
{
if (indexAttribute.Name == null)
{
Expand Down
47 changes: 23 additions & 24 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -980,9 +980,8 @@ public virtual bool IsIgnored([NotNull] string name, ConfigurationSource? config
{
Check.NotEmpty(name, nameof(name));

var ignoredConfigurationSource = Metadata.FindIgnoredConfigurationSource(name);
return !configurationSource.HasValue
|| !configurationSource.Value.Overrides(ignoredConfigurationSource);
return configurationSource != ConfigurationSource.Explicit
&& !configurationSource.OverridesStrictly(Metadata.FindIgnoredConfigurationSource(name));
}

/// <summary>
Expand Down Expand Up @@ -1088,7 +1087,15 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura
if (derivedNavigation != null)
{
var foreignKey = derivedNavigation.ForeignKey;
if (configurationSource != foreignKey.GetConfigurationSource())
if (foreignKey.GetConfigurationSource() != derivedNavigation.GetConfigurationSource())
{
if (derivedNavigation.GetConfigurationSource() != ConfigurationSource.Explicit)
{
foreignKey.Builder.HasNavigation(
(MemberInfo)null, derivedNavigation.IsOnDependent, configurationSource);
}
}
else if (foreignKey.GetConfigurationSource() != ConfigurationSource.Explicit)
{
foreignKey.DeclaringEntityType.Builder.HasNoRelationship(
foreignKey, configurationSource);
Expand All @@ -1099,7 +1106,8 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura
var derivedProperty = derivedType.FindDeclaredProperty(name);
if (derivedProperty != null)
{
derivedType.Builder.RemoveProperty(derivedProperty, configurationSource, canOverrideSameSource: false);
derivedType.Builder.RemoveProperty(
derivedProperty, configurationSource, canOverrideSameSource: configurationSource != ConfigurationSource.Explicit);
}
else
{
Expand All @@ -1115,7 +1123,8 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura
inverse.DeclaringEntityType.RemoveSkipNavigation(inverse);
}

if (configurationSource.OverridesStrictly(skipNavigation.GetConfigurationSource()))
if (configurationSource.Overrides(skipNavigation.GetConfigurationSource())
&& skipNavigation.GetConfigurationSource() != ConfigurationSource.Explicit)
{
derivedType.RemoveSkipNavigation(skipNavigation);
}
Expand All @@ -1124,7 +1133,8 @@ public virtual InternalEntityTypeBuilder Ignore([NotNull] string name, Configura
{
var derivedServiceProperty = derivedType.FindDeclaredServiceProperty(name);
if (derivedServiceProperty != null
&& configurationSource.OverridesStrictly(derivedServiceProperty.GetConfigurationSource()))
&& configurationSource.Overrides(derivedServiceProperty.GetConfigurationSource())
&& derivedServiceProperty.GetConfigurationSource() != ConfigurationSource.Explicit)
{
derivedType.RemoveServiceProperty(name);
}
Expand Down Expand Up @@ -1157,7 +1167,6 @@ private bool CanIgnore(string name, ConfigurationSource configurationSource, boo
var navigation = Metadata.FindNavigation(name);
if (navigation != null)
{
var foreignKey = navigation.ForeignKey;
if (navigation.DeclaringEntityType != Metadata)
{
if (shouldThrow)
Expand All @@ -1170,16 +1179,7 @@ private bool CanIgnore(string name, ConfigurationSource configurationSource, boo
return false;
}

var navigationConfigurationSource = navigation.GetConfigurationSource();
if (foreignKey.GetConfigurationSource() != navigationConfigurationSource)
{
if (!configurationSource.Overrides(navigationConfigurationSource))
{
return false;
}
}
else if (configurationSource != ConfigurationSource.Explicit
&& !configurationSource.OverridesStrictly(foreignKey.GetConfigurationSource()))
if (!configurationSource.Overrides(navigation.GetConfigurationSource()))
{
return false;
}
Expand All @@ -1202,7 +1202,7 @@ private bool CanIgnore(string name, ConfigurationSource configurationSource, boo
}

if (!property.DeclaringEntityType.Builder.CanRemoveProperty(
property, configurationSource, canOverrideSameSource: configurationSource == ConfigurationSource.Explicit))
property, configurationSource, canOverrideSameSource: true))
{
return false;
}
Expand All @@ -1224,8 +1224,7 @@ private bool CanIgnore(string name, ConfigurationSource configurationSource, boo
return false;
}

if (configurationSource != ConfigurationSource.Explicit
&& !configurationSource.OverridesStrictly(skipNavigation.GetConfigurationSource()))
if (!configurationSource.Overrides(skipNavigation.GetConfigurationSource()))
{
return false;
}
Expand All @@ -1247,8 +1246,7 @@ private bool CanIgnore(string name, ConfigurationSource configurationSource, boo
return false;
}

if (configurationSource != ConfigurationSource.Explicit
&& !configurationSource.OverridesStrictly(serviceProperty.GetConfigurationSource()))
if (!configurationSource.Overrides(serviceProperty.GetConfigurationSource()))
{
return false;
}
Expand Down Expand Up @@ -1690,8 +1688,9 @@ List<T> FindConflictingMembers<T>(
foreach (var member in derivedMembers)
{
ConfigurationSource? baseConfigurationSource = null;
if (!member.GetConfigurationSource().Overrides(
if ((!member.GetConfigurationSource().OverridesStrictly(
baseEntityType.FindIgnoredConfigurationSource(member.Name))
&& member.GetConfigurationSource() != ConfigurationSource.Explicit)
|| (baseMemberNames.TryGetValue(member.Name, out baseConfigurationSource)
&& baseConfigurationSource.Overrides(member.GetConfigurationSource())
&& !compatibleWithBaseMember(member)))
Expand Down
20 changes: 11 additions & 9 deletions test/EFCore.Tests/Metadata/Internal/InternalEntityTypeBuilderTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1905,9 +1905,9 @@ private void VerifyIgnoreMember(

var expectedAdded = ignoredOnType == typeof(ExtraSpecialOrder)
|| (addConfigurationSource.Overrides(ignoreConfigurationSource)
&& (ignoreConfigurationSource != ConfigurationSource.Explicit
|| ignoredOnType != typeof(SpecialOrder)
|| ignoredFirst));
&& (ignoreConfigurationSource != addConfigurationSource
|| (ignoreConfigurationSource == ConfigurationSource.Explicit
&& (ignoredFirst || ignoredOnType != typeof(SpecialOrder)))));

var expectedIgnored = (ignoredOnType != typeof(SpecialOrder)
|| !expectedAdded)
Expand Down Expand Up @@ -1962,12 +1962,14 @@ private void VerifyIgnoreMember(

Assert.True(unignoreMember(ignoredEntityTypeBuilder));
}

Assert.Equal(expectedAdded, findMember(addedEntityTypeBuilder));
Assert.Equal(
expectedIgnored,
ignoredEntityTypeBuilder.Metadata.FindDeclaredIgnoredConfigurationSource(memberToIgnore)
== ignoreConfigurationSource);
else
{
Assert.Equal(expectedAdded, findMember(addedEntityTypeBuilder));
Assert.Equal(
expectedIgnored,
ignoredEntityTypeBuilder.Metadata.FindDeclaredIgnoredConfigurationSource(memberToIgnore)
== ignoreConfigurationSource);
}
}

private void ConfigureOrdersHierarchy(InternalModelBuilder modelBuilder)
Expand Down