Skip to content

Commit

Permalink
Allow removing the discriminator for entity types that are not part o…
Browse files Browse the repository at this point in the history
…f a hierarchy.

Part of #12086
  • Loading branch information
AndriySvyryd committed Jun 14, 2019
1 parent c1f72a5 commit 22c52f0
Show file tree
Hide file tree
Showing 28 changed files with 148 additions and 84 deletions.
9 changes: 7 additions & 2 deletions azure-pipelines.yml
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ variables:
value: true
- name: _DotNetArtifactsCategory
value: ENTITYFRAMEWORKCORE
- name: _CosmosConnectionUrl
value: 'https://ef-nightly-test.documents.azure.com:443/'

trigger:
- master
Expand Down Expand Up @@ -77,6 +79,8 @@ jobs:
'@
displayName: Cleanup databases
- script: eng\common\cibuild.cmd -configuration $(_BuildConfig) -prepareMachine $(_InternalBuildArgs)
env:
Test__Cosmos__Sql__DefaultConnection: $(_CosmosConnectionUrl)
name: Build
- task: PublishBuildArtifacts@1
displayName: Upload artifacts
Expand All @@ -92,15 +96,14 @@ jobs:
vmImage: macOS-10.13
variables:
- ${{ if and(eq(variables['System.TeamProject'], 'public'), eq(variables['system.pullrequest.isfork'], false), notin(variables['Build.Reason'], 'IndividualCI', 'BatchedCI', 'PullRequest')) }}:
- _CosmosConnection: https://ef-nightly-test.documents.azure.com:443/
- _CosmosToken: $(ef-nightly-cosmos-key)
steps:
- bash: brew install libspatialite
displayName: Install SpatiaLite
continueOnError: true
- script: eng/common/cibuild.sh --configuration $(_BuildConfig) --binaryLog --prepareMachine
env:
Test__Cosmos__Sql__DefaultConnection: $(_CosmosConnection)
Test__Cosmos__Sql__DefaultConnection: $(_CosmosConnectionUrl)
Test__Cosmos__Sql__AuthToken: $(_CosmosToken)
name: Build
continueOnError: true
Expand All @@ -113,4 +116,6 @@ jobs:
displayName: Install SpatiaLite
continueOnError: true
- script: eng/common/cibuild.sh --configuration $(_BuildConfig) --prepareMachine
env:
Test__Cosmos__Sql__DefaultConnection: $(_CosmosConnectionUrl)
name: Build
33 changes: 20 additions & 13 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -859,21 +859,19 @@ public virtual DataBuilder HasData([NotNull] IEnumerable<object> data)
}

/// <summary>
/// Configures the discriminator column used to identify which entity type each row in a table represents
/// when an inheritance hierarchy is mapped to a single table in a relational database.
/// Configures the discriminator property used to identify the entity type in the store.
/// </summary>
/// <returns> A builder that allows the discriminator column to be configured. </returns>
/// <returns> A builder that allows the discriminator property to be configured. </returns>
public virtual DiscriminatorBuilder HasDiscriminator()
=> Builder.DiscriminatorBuilder(
Builder.GetOrCreateDiscriminatorProperty(null, null, true), ConfigurationSource.Explicit);

/// <summary>
/// Configures the discriminator column used to identify which entity type each row in a table represents
/// when an inheritance hierarchy is mapped to a single table in a relational database.
/// Configures the discriminator property used to identify the entity type in the store.
/// </summary>
/// <param name="name"> The name of the discriminator column. </param>
/// <param name="type"> The type of values stored in the discriminator column. </param>
/// <returns> A builder that allows the discriminator column to be configured. </returns>
/// <param name="name"> The name of the discriminator property. </param>
/// <param name="type"> The type of values stored in the discriminator property. </param>
/// <returns> A builder that allows the discriminator property to be configured. </returns>
public virtual DiscriminatorBuilder HasDiscriminator(
[NotNull] string name,
[NotNull] Type type)
Expand All @@ -885,12 +883,11 @@ public virtual DiscriminatorBuilder HasDiscriminator(
}

/// <summary>
/// Configures the discriminator column used to identify which entity type each row in a table represents
/// when an inheritance hierarchy is mapped to a single table in a relational database.
/// Configures the discriminator property used to identify the entity type in the store.
/// </summary>
/// <typeparam name="TDiscriminator"> The type of values stored in the discriminator column. </typeparam>
/// <param name="name"> The name of the discriminator column. </param>
/// <returns> A builder that allows the discriminator column to be configured. </returns>
/// <typeparam name="TDiscriminator"> The type of values stored in the discriminator property. </typeparam>
/// <param name="name"> The name of the discriminator property. </param>
/// <returns> A builder that allows the discriminator property to be configured. </returns>
public virtual DiscriminatorBuilder<TDiscriminator> HasDiscriminator<TDiscriminator>([NotNull] string name)
{
Check.NotEmpty(name, nameof(name));
Expand All @@ -899,6 +896,16 @@ public virtual DiscriminatorBuilder<TDiscriminator> HasDiscriminator<TDiscrimina
Builder.DiscriminatorBuilder(Property(typeof(TDiscriminator), name).GetInfrastructure(), ConfigurationSource.Explicit));
}

/// <summary>
/// Configures the entity type as having no discriminator property.
/// </summary>
/// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
public virtual EntityTypeBuilder HasNoDiscriminator()
{
Builder.HasNoDeclaredDiscriminator(ConfigurationSource.Explicit);
return this;
}

#region Hidden System.Object members

/// <summary>
Expand Down
14 changes: 10 additions & 4 deletions src/EFCore/Metadata/Builders/EntityTypeBuilder`.cs
Original file line number Diff line number Diff line change
Expand Up @@ -756,15 +756,14 @@ public virtual DataBuilder<TEntity> HasData([NotNull] IEnumerable<TEntity> data)
}

/// <summary>
/// Configures the discriminator column used to identify which entity type each row in a table represents
/// when an inheritance hierarchy is mapped to a single table in a relational database.
/// Configures the discriminator property used to identify the entity type in the store.
/// </summary>
/// <typeparam name="TDiscriminator"> The type of values stored in the discriminator column. </typeparam>
/// <typeparam name="TDiscriminator"> The type of values stored in the discriminator property. </typeparam>
/// <param name="propertyExpression">
/// A lambda expression representing the property to be used as the discriminator (
/// <c>blog => blog.Discriminator</c>).
/// </param>
/// <returns> A builder that allows the discriminator column to be configured. </returns>
/// <returns> A builder that allows the discriminator property to be configured. </returns>
public virtual DiscriminatorBuilder<TDiscriminator> HasDiscriminator<TDiscriminator>(
[NotNull] Expression<Func<TEntity, TDiscriminator>> propertyExpression)
{
Expand All @@ -774,6 +773,13 @@ public virtual DiscriminatorBuilder<TDiscriminator> HasDiscriminator<TDiscrimina
Builder.DiscriminatorBuilder(Property(propertyExpression).GetInfrastructure(), ConfigurationSource.Explicit));
}

/// <summary>
/// Configures the entity type as having no discriminator property.
/// </summary>
/// <returns> The same builder instance so that multiple configuration calls can be chained. </returns>
public new virtual EntityTypeBuilder<TEntity> HasNoDiscriminator()
=> (EntityTypeBuilder<TEntity>)base.HasNoDiscriminator();

private InternalEntityTypeBuilder Builder => this.GetInfrastructure();
}
}
16 changes: 10 additions & 6 deletions src/EFCore/Metadata/Internal/EntityType.cs
Original file line number Diff line number Diff line change
Expand Up @@ -2519,23 +2519,27 @@ public virtual void SetDiscriminatorProperty([CanBeNull] IProperty property, Con
{
CheckDiscriminatorProperty(property);

if (property != null
&& !property.ClrType.IsInstanceOfType(this.GetDiscriminatorValue()))
if ((property == null
|| !property.ClrType.IsInstanceOfType(this.GetDiscriminatorValue())))
{
foreach (var derivedType in GetDerivedTypesInclusive())
((IMutableEntityType)this).RemoveDiscriminatorValue();
if (BaseType == null)
{
((IMutableEntityType)derivedType).RemoveDiscriminatorValue();
foreach (var derivedType in GetDerivedTypes())
{
((IMutableEntityType)derivedType).RemoveDiscriminatorValue();
}
}
}

this.SetOrRemoveAnnotation(CoreAnnotationNames.DiscriminatorProperty, property?.Name, configurationSource);
SetAnnotation(CoreAnnotationNames.DiscriminatorProperty, property?.Name, configurationSource);
}

private void CheckDiscriminatorProperty(IProperty property)
{
if (property != null)
{
if (this != RootType())
if (BaseType != null)
{
throw new InvalidOperationException(
CoreStrings.DiscriminatorPropertyMustBeOnRoot(this.DisplayName()));
Expand Down
70 changes: 34 additions & 36 deletions src/EFCore/Metadata/Internal/InternalEntityTypeBuilder.cs
Original file line number Diff line number Diff line change
Expand Up @@ -3374,9 +3374,38 @@ public virtual DiscriminatorBuilder DiscriminatorBuilder(
discriminatorPropertyBuilder = rootTypeBuilder.Property(
discriminatorProperty.ClrType, discriminatorProperty.Name, null, ConfigurationSource.Convention);

RemoveUnusedDiscriminatorProperty(discriminatorProperty, configurationSource);

rootTypeBuilder.Metadata.SetDiscriminatorProperty(discriminatorProperty, configurationSource);
discriminatorPropertyBuilder.IsRequired(true, configurationSource);
discriminatorPropertyBuilder.HasValueGenerator(DiscriminatorValueGenerator.Factory, configurationSource);

return new DiscriminatorBuilder(Metadata);
}

public virtual InternalEntityTypeBuilder HasNoDeclaredDiscriminator(ConfigurationSource configurationSource)
{
if (Metadata[CoreAnnotationNames.DiscriminatorProperty] != null
&& !configurationSource.Overrides(Metadata.GetDiscriminatorPropertyConfigurationSource()))
{
return null;
}

if (Metadata.BaseType == null)
{
RemoveUnusedDiscriminatorProperty(null, configurationSource);
}

Metadata.SetDiscriminatorProperty(null, configurationSource);

return this;
}

private void RemoveUnusedDiscriminatorProperty(Property newDiscriminatorProperty, ConfigurationSource configurationSource)
{
var oldDiscriminatorProperty = Metadata.GetDiscriminatorProperty() as Property;
if (oldDiscriminatorProperty?.Builder != null
&& oldDiscriminatorProperty != discriminatorProperty)
&& oldDiscriminatorProperty != newDiscriminatorProperty)
{
oldDiscriminatorProperty.DeclaringEntityType.Builder.RemoveUnusedShadowProperties(
new[]
Expand All @@ -3390,23 +3419,16 @@ public virtual DiscriminatorBuilder DiscriminatorBuilder(
oldDiscriminatorProperty.Builder.HasValueGenerator((Type)null, configurationSource);
}
}

rootTypeBuilder.Metadata.SetDiscriminatorProperty(discriminatorProperty, configurationSource);
discriminatorPropertyBuilder.IsRequired(true, configurationSource);
discriminatorPropertyBuilder.HasValueGenerator(DiscriminatorValueGenerator.Factory, configurationSource);

return new DiscriminatorBuilder(Metadata);
}

private bool CanSetDiscriminator(
IProperty discriminatorProperty,
string name,
Type discriminatorType,
bool fromDataAnnotation)
=> discriminatorProperty == null
|| ((name != null || discriminatorType != null)
&& (name == null || discriminatorProperty.Name == name)
&& (discriminatorType == null || discriminatorProperty.ClrType == discriminatorType))
=> (name == null && discriminatorType == null)
|| ((name == null || discriminatorProperty?.Name == name)
&& (discriminatorType == null || discriminatorProperty?.ClrType == discriminatorType))
|| (fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention)
.Overrides(Metadata.GetDiscriminatorPropertyConfigurationSource());

Expand Down Expand Up @@ -3745,31 +3767,7 @@ IConventionDiscriminatorBuilder IConventionEntityTypeBuilder.HasDiscriminator(Me

/// <inheritdoc />
IConventionEntityTypeBuilder IConventionEntityTypeBuilder.HasNoDeclaredDiscriminator(bool fromDataAnnotation)
{
var discriminatorName = (string)Metadata[CoreAnnotationNames.DiscriminatorProperty];
if (discriminatorName == null)
{
return this;
}

var discriminatorProperty = Metadata.FindProperty(discriminatorName);
if (discriminatorProperty != null)
{
if (!CanSetDiscriminator(discriminatorProperty, null, null, fromDataAnnotation))
{
return null;
}

discriminatorProperty.DeclaringEntityType.Builder.RemoveUnusedShadowProperties(
new[]
{
discriminatorProperty
});
}

Metadata.SetDiscriminatorProperty(null, fromDataAnnotation);
return this;
}
=> HasNoDeclaredDiscriminator(fromDataAnnotation ? ConfigurationSource.DataAnnotation : ConfigurationSource.Convention);

/// <inheritdoc />
bool IConventionEntityTypeBuilder.CanSetDiscriminator(string name, bool fromDataAnnotation)
Expand Down
27 changes: 27 additions & 0 deletions test/EFCore.Cosmos.Tests/Metadata/CosmosBuilderExtensionsTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,33 @@ public void Default_container_name_is_used_if_not_set()
Assert.Equal("db1", entityType.GetCosmosContainerName());
}

[ConditionalFact]
public void Default_discriminator_can_be_removed()
{
var modelBuilder = CreateConventionModelBuilder();

modelBuilder.Entity<Customer>();

var entityType = modelBuilder.Model.FindEntityType(typeof(Customer));

Assert.Equal("Discriminator", entityType.GetDiscriminatorProperty().Name);
Assert.Equal(nameof(Customer), entityType.GetDiscriminatorValue());

modelBuilder.Entity<Customer>().HasNoDiscriminator();

Assert.Null(entityType.GetDiscriminatorProperty());
Assert.Null(entityType.GetDiscriminatorValue());

modelBuilder.Entity<Customer>().HasBaseType<object>();

Assert.Equal("Discriminator", entityType.GetDiscriminatorProperty().Name);
Assert.Equal(nameof(Customer), entityType.GetDiscriminatorValue());

modelBuilder.Entity<Customer>().HasBaseType((string)null);

Assert.Null(entityType.GetDiscriminatorProperty());
}

protected virtual ModelBuilder CreateConventionModelBuilder() => CosmosTestHelpers.Instance.CreateConventionBuilder();

private class Customer
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
using Microsoft.EntityFrameworkCore.Diagnostics;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -21,7 +22,7 @@

#pragma warning disable RCS1213 // Remove unused member declaration.

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class BackingFieldConventionTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@

using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class BaseTypeDiscoveryConventionTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
// ReSharper disable ClassNeverInstantiated.Local
// ReSharper disable CollectionNeverUpdated.Local
// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class CascadeDeleteConventionTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
using Microsoft.EntityFrameworkCore.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.Extensions.DependencyInjection;
Expand All @@ -21,7 +22,7 @@
// ReSharper disable MemberCanBePrivate.Local
// ReSharper disable MemberHidesStaticFromOuterClass
// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class ConstructorBindingConventionTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
using Xunit;

// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class ConventionDispatcherTest
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@

using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Internal;
using Microsoft.EntityFrameworkCore.TestUtilities;
using Microsoft.Extensions.DependencyInjection;
using Xunit;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions.Internal
namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
public class DerivedTypeDiscoveryConventionTest
{
Expand Down
Loading

0 comments on commit 22c52f0

Please sign in to comment.