Skip to content

Commit

Permalink
Cosmos: Set ValueGenerated.Never for most integer properties
Browse files Browse the repository at this point in the history
Fixes #25167
  • Loading branch information
AndriySvyryd authored Sep 23, 2021
1 parent 059433e commit 48b7672
Show file tree
Hide file tree
Showing 6 changed files with 138 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,102 @@
// Licensed to the .NET Foundation under one or more agreements.
// The .NET Foundation licenses this file to you under the MIT license.

using System;
using System.Linq;
using Microsoft.EntityFrameworkCore.Cosmos.Metadata.Internal;
using Microsoft.EntityFrameworkCore.Metadata.Builders;
using Microsoft.EntityFrameworkCore.Metadata.Conventions.Infrastructure;

namespace Microsoft.EntityFrameworkCore.Metadata.Conventions
{
/// <summary>
/// A convention that configures store value generation as <see cref="ValueGenerated.OnAdd" /> on properties that are
/// part of the primary key and not part of any foreign keys or were configured to have a database default value.
/// </summary>
/// <remarks>
/// See <see href="https://aka.ms/efcore-docs-conventions">Model building conventions</see> and
/// <see href="https://aka.ms/efcore-docs-value-generation">EF Core value generation</see> for more information.
/// </remarks>
public class CosmosValueGenerationConvention :
ValueGenerationConvention,
IEntityTypeAnnotationChangedConvention
{
/// <summary>
/// Creates a new instance of <see cref="CosmosValueGenerationConvention" />.
/// </summary>
/// <param name="dependencies"> Parameter object containing dependencies for this convention. </param>
public CosmosValueGenerationConvention(
ProviderConventionSetBuilderDependencies dependencies)
: base(dependencies)
{
}

/// <summary>
/// Called after an annotation is changed on an entity type.
/// </summary>
/// <param name="entityTypeBuilder"> The builder for the entity type. </param>
/// <param name="name"> The annotation name. </param>
/// <param name="annotation"> The new annotation. </param>
/// <param name="oldAnnotation"> The old annotation. </param>
/// <param name="context"> Additional information associated with convention execution. </param>
public virtual void ProcessEntityTypeAnnotationChanged(
IConventionEntityTypeBuilder entityTypeBuilder,
string name,
IConventionAnnotation? annotation,
IConventionAnnotation? oldAnnotation,
IConventionContext<IConventionAnnotation> context)
{
if (name != CosmosAnnotationNames.ContainerName
|| (annotation == null) == (oldAnnotation == null))
{
return;
}

var primaryKey = entityTypeBuilder.Metadata.FindPrimaryKey();
if (primaryKey == null)
{
return;
}

foreach (var property in primaryKey.Properties)
{
property.Builder.ValueGenerated(GetValueGenerated(property));
}
}

/// <summary>
/// Returns the store value generation strategy to set for the given property.
/// </summary>
/// <param name="property"> The property. </param>
/// <returns> The store value generation strategy to set for the given property. </returns>
protected override ValueGenerated? GetValueGenerated(IConventionProperty property)
{
var entityType = property.DeclaringEntityType;
var propertyType = property.ClrType.UnwrapNullableType();
if (propertyType == typeof(int))
{
var ownership = entityType.FindOwnership();
if (ownership != null
&& !ownership.IsUnique
&& !entityType.IsDocumentRoot())
{
var pk = property.FindContainingPrimaryKey();
if (pk != null
&& !ownership.Properties.Contains(property)
&& pk.Properties.Count == ownership.Properties.Count + 1
&& ownership.Properties.All(fkProperty => pk.Properties.Contains(fkProperty)))
{
return base.GetValueGenerated(property);
}
}
}

if (propertyType != typeof(Guid))
{
return null;
}

return base.GetValueGenerated(property);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,9 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(conventionSet.EntityTypeRemovedConventions, (DiscriminatorConvention)discriminatorConvention);
ReplaceConvention(conventionSet.EntityTypeRemovedConventions, inversePropertyAttributeConvention);

ValueGenerationConvention valueGenerationConvention = new CosmosValueGenerationConvention(Dependencies);
conventionSet.EntityTypeBaseTypeChangedConventions.Add(storeKeyConvention);
ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, valueGenerationConvention);
ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, (DiscriminatorConvention)discriminatorConvention);
ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, keyDiscoveryConvention);
ReplaceConvention(conventionSet.EntityTypeBaseTypeChangedConventions, inversePropertyAttributeConvention);
Expand All @@ -71,26 +73,31 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(conventionSet.EntityTypeMemberIgnoredConventions, relationshipDiscoveryConvention);

conventionSet.EntityTypePrimaryKeyChangedConventions.Add(storeKeyConvention);
ReplaceConvention(conventionSet.EntityTypePrimaryKeyChangedConventions, valueGenerationConvention);

conventionSet.KeyAddedConventions.Add(storeKeyConvention);

conventionSet.KeyRemovedConventions.Add(storeKeyConvention);
ReplaceConvention(conventionSet.KeyRemovedConventions, keyDiscoveryConvention);

ReplaceConvention(conventionSet.ForeignKeyAddedConventions, keyDiscoveryConvention);
ReplaceConvention(conventionSet.ForeignKeyAddedConventions, valueGenerationConvention);

ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, relationshipDiscoveryConvention);
conventionSet.ForeignKeyRemovedConventions.Add(discriminatorConvention);
conventionSet.ForeignKeyRemovedConventions.Add(storeKeyConvention);
ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, keyDiscoveryConvention);
ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, valueGenerationConvention);

ReplaceConvention(conventionSet.ForeignKeyPropertiesChangedConventions, keyDiscoveryConvention);
ReplaceConvention(conventionSet.ForeignKeyPropertiesChangedConventions, valueGenerationConvention);

ReplaceConvention(conventionSet.ForeignKeyUniquenessChangedConventions, keyDiscoveryConvention);

conventionSet.ForeignKeyOwnershipChangedConventions.Add(discriminatorConvention);
conventionSet.ForeignKeyOwnershipChangedConventions.Add(storeKeyConvention);
ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, keyDiscoveryConvention);
ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, valueGenerationConvention);
ReplaceConvention(conventionSet.ForeignKeyOwnershipChangedConventions, relationshipDiscoveryConvention);

ReplaceConvention(conventionSet.ForeignKeyNullNavigationSetConventions, relationshipDiscoveryConvention);
Expand All @@ -111,6 +118,7 @@ public override ConventionSet CreateConventionSet()

conventionSet.EntityTypeAnnotationChangedConventions.Add(discriminatorConvention);
conventionSet.EntityTypeAnnotationChangedConventions.Add(storeKeyConvention);
conventionSet.EntityTypeAnnotationChangedConventions.Add((CosmosValueGenerationConvention)valueGenerationConvention);
conventionSet.EntityTypeAnnotationChangedConventions.Add((CosmosKeyDiscoveryConvention)keyDiscoveryConvention);
conventionSet.EntityTypeAnnotationChangedConventions.Add((CosmosManyToManyJoinEntityTypeConvention)manyToManyJoinEntityTypeConvention);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public override ConventionSet CreateConventionSet()
ReplaceConvention(conventionSet.EntityTypePrimaryKeyChangedConventions, valueGenerationConvention);

ReplaceConvention(conventionSet.ForeignKeyAddedConventions, valueGenerationConvention);

ReplaceConvention(conventionSet.ForeignKeyRemovedConventions, valueGenerationConvention);

conventionSet.PropertyFieldChangedConventions.Add(relationalColumnAttributeConvention);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ protected override void Mapping_throws_for_non_ignored_array()
Assert.NotNull(property.FindTypeMapping());
}

[ConditionalFact]
public override void Properties_can_have_provider_type_set_for_type()
{
var modelBuilder = CreateModelBuilder(c => c.Properties<string>().HaveConversion<byte[]>());
Expand All @@ -69,6 +68,33 @@ public override void Properties_can_have_provider_type_set_for_type()
Assert.Same(typeof(byte[]), entityType.FindProperty("Strange").GetProviderClrType());
}

public override void Properties_can_be_set_to_generate_values_on_Add()
{
var modelBuilder = CreateModelBuilder();

modelBuilder.Entity<Quarks>(
b =>
{
b.HasKey(e => e.Id);
b.Property(e => e.Up).ValueGeneratedOnAddOrUpdate();
b.Property(e => e.Down).ValueGeneratedNever();
b.Property<int>("Charm").Metadata.ValueGenerated = ValueGenerated.OnUpdateSometimes;
b.Property<string>("Strange").ValueGeneratedNever();
b.Property<int>("Top").ValueGeneratedOnAddOrUpdate();
b.Property<string>("Bottom").ValueGeneratedOnUpdate();
});

var model = modelBuilder.FinalizeModel();
var entityType = model.FindEntityType(typeof(Quarks));
Assert.Equal(ValueGenerated.Never, entityType.FindProperty(Customer.IdProperty.Name).ValueGenerated);
Assert.Equal(ValueGenerated.OnAddOrUpdate, entityType.FindProperty("Up").ValueGenerated);
Assert.Equal(ValueGenerated.Never, entityType.FindProperty("Down").ValueGenerated);
Assert.Equal(ValueGenerated.OnUpdateSometimes, entityType.FindProperty("Charm").ValueGenerated);
Assert.Equal(ValueGenerated.Never, entityType.FindProperty("Strange").ValueGenerated);
Assert.Equal(ValueGenerated.OnAddOrUpdate, entityType.FindProperty("Top").ValueGenerated);
Assert.Equal(ValueGenerated.OnUpdate, entityType.FindProperty("Bottom").ValueGenerated);
}

[ConditionalFact]
public virtual void Partition_key_is_added_to_the_keys()
{
Expand Down
4 changes: 0 additions & 4 deletions test/EFCore.Tests/ModelBuilding/OneToManyTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -1474,10 +1474,6 @@ public virtual void Explicit_principal_key_is_not_replaced_with_new_primary_key(
Assert.Contains(nonPrimaryPrincipalKey, principalType.GetKeys());
var oldKeyProperty = principalType.FindProperty(nameof(BigMak.Id));
var newKeyProperty = principalType.FindProperty(nameof(BigMak.AlternateKey));
Assert.False(oldKeyProperty.RequiresValueGenerator());
Assert.Equal(ValueGenerated.Never, oldKeyProperty.ValueGenerated);
Assert.True(newKeyProperty.RequiresValueGenerator());
Assert.Equal(ValueGenerated.OnAdd, newKeyProperty.ValueGenerated);
Assert.Same(dependentKey, dependentType.FindPrimaryKey());

Assert.Equal(dependentType.GetForeignKeys().Count(), dependentType.GetIndexes().Count());
Expand Down
4 changes: 0 additions & 4 deletions test/EFCore.Tests/ModelBuilding/OwnedTypesTestBase.cs
Original file line number Diff line number Diff line change
Expand Up @@ -679,7 +679,6 @@ public virtual void Can_configure_owned_type_collection_without_explicit_key()
Assert.Single(owned.GetForeignKeys());
var pk = owned.FindPrimaryKey();
Assert.Equal(new[] { nameof(Order.CustomerId), nameof(Order.OrderId) }, pk.Properties.Select(p => p.Name));
Assert.Equal(ValueGenerated.OnAdd, pk.Properties.Last().ValueGenerated);
Assert.Empty(owned.GetIndexes());

var chainedOwnership = owned.FindNavigation(nameof(Order.Products)).ForeignKey;
Expand All @@ -690,7 +689,6 @@ public virtual void Can_configure_owned_type_collection_without_explicit_key()
Assert.Equal(new[] { "OrderCustomerId", "OrderId" }, chainedOwnership.Properties.Select(p => p.Name));
var chainedPk = chainedOwned.FindPrimaryKey();
Assert.Equal(new[] { "OrderCustomerId", "OrderId", nameof(Product.Id) }, chainedPk.Properties.Select(p => p.Name));
Assert.Equal(ValueGenerated.OnAdd, chainedPk.Properties.Last().ValueGenerated);
Assert.Empty(chainedOwned.GetIndexes());

Assert.Equal(4, model.GetEntityTypes().Count());
Expand Down Expand Up @@ -728,7 +726,6 @@ public virtual void Can_configure_owned_type_collection_without_explicit_key_or_
Assert.Single(owned.GetForeignKeys());
var pk = owned.FindPrimaryKey();
Assert.Equal(new[] { nameof(Order.CustomerId), "Id" }, pk.Properties.Select(p => p.Name));
Assert.Equal(ValueGenerated.OnAdd, pk.Properties.Last().ValueGenerated);
Assert.Empty(owned.GetIndexes());

var chainedOwnership = owned.FindNavigation(nameof(Order.Products)).ForeignKey;
Expand All @@ -739,7 +736,6 @@ public virtual void Can_configure_owned_type_collection_without_explicit_key_or_
Assert.Equal(new[] { "OrderCustomerId", "OrderId" }, chainedOwnership.Properties.Select(p => p.Name));
var chainedPk = chainedOwned.FindPrimaryKey();
Assert.Equal(new[] { "OrderCustomerId", "OrderId", "Id1" }, chainedPk.Properties.Select(p => p.Name));
Assert.Equal(ValueGenerated.OnAdd, chainedPk.Properties.Last().ValueGenerated);
Assert.Empty(chainedOwned.GetIndexes());

Assert.Equal(4, model.GetEntityTypes().Count());
Expand Down

0 comments on commit 48b7672

Please sign in to comment.