Skip to content

Commit ac91e9a

Browse files
rojiroji
authored
Stop eagerly throwing when setting value generation strategy (#3147)
Closes #3141
1 parent 18008c9 commit ac91e9a

File tree

4 files changed

+77
-74
lines changed

4 files changed

+77
-74
lines changed

‎src/EFCore.PG/Extensions/BuilderExtensions/NpgsqlPropertyBuilderExtensions.cs‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -425,6 +425,26 @@ public static PropertyBuilder<TProperty> UseIdentityColumn<TProperty>(
425425
return null;
426426
}
427427

428+
/// <summary>
429+
/// Returns a value indicating whether the given value can be set as the value generation strategy for a particular table.
430+
/// </summary>
431+
/// <param name="propertyBuilder">The builder for the property being configured.</param>
432+
/// <param name="valueGenerationStrategy">The value generation strategy.</param>
433+
/// <param name="storeObject">The table identifier.</param>
434+
/// <param name="fromDataAnnotation">Indicates whether the configuration was specified using a data annotation.</param>
435+
/// <returns><see langword="true" /> if the given value can be set as the default value generation strategy.</returns>
436+
public static bool CanSetValueGenerationStrategy(
437+
this IConventionPropertyBuilder propertyBuilder,
438+
NpgsqlValueGenerationStrategy? valueGenerationStrategy,
439+
in StoreObjectIdentifier storeObject,
440+
bool fromDataAnnotation = false)
441+
=> propertyBuilder.Metadata.FindOverrides(storeObject)?.Builder
442+
.CanSetAnnotation(
443+
NpgsqlAnnotationNames.ValueGenerationStrategy,
444+
valueGenerationStrategy,
445+
fromDataAnnotation)
446+
?? true;
447+
428448
/// <summary>
429449
/// Returns a value indicating whether the given value can be set as the value generation strategy.
430450
/// </summary>
@@ -436,14 +456,8 @@ public static bool CanSetValueGenerationStrategy(
436456
this IConventionPropertyBuilder propertyBuilder,
437457
NpgsqlValueGenerationStrategy? valueGenerationStrategy,
438458
bool fromDataAnnotation = false)
439-
{
440-
Check.NotNull(propertyBuilder, nameof(propertyBuilder));
441-
442-
return (valueGenerationStrategy is null
443-
|| NpgsqlPropertyExtensions.IsCompatibleWithValueGeneration(propertyBuilder.Metadata))
444-
&& propertyBuilder.CanSetAnnotation(
445-
NpgsqlAnnotationNames.ValueGenerationStrategy, valueGenerationStrategy, fromDataAnnotation);
446-
}
459+
=> propertyBuilder.CanSetAnnotation(
460+
NpgsqlAnnotationNames.ValueGenerationStrategy, valueGenerationStrategy, fromDataAnnotation);
447461

448462
#endregion General value generation strategy
449463

‎src/EFCore.PG/Extensions/MetadataExtensions/NpgsqlPropertyExtensions.cs‎

Lines changed: 6 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -589,11 +589,7 @@ private static NpgsqlValueGenerationStrategy GetDefaultValueGenerationStrategy(
589589
public static void SetValueGenerationStrategy(
590590
this IMutableProperty property,
591591
NpgsqlValueGenerationStrategy? value)
592-
{
593-
CheckValueGenerationStrategy(property, value);
594-
595-
property.SetOrRemoveAnnotation(NpgsqlAnnotationNames.ValueGenerationStrategy, value);
596-
}
592+
=> property.SetOrRemoveAnnotation(NpgsqlAnnotationNames.ValueGenerationStrategy, value);
597593

598594
/// <summary>
599595
/// Sets the <see cref="NpgsqlValueGenerationStrategy" /> to use for the property.
@@ -605,13 +601,8 @@ public static void SetValueGenerationStrategy(
605601
this IConventionProperty property,
606602
NpgsqlValueGenerationStrategy? value,
607603
bool fromDataAnnotation = false)
608-
{
609-
CheckValueGenerationStrategy(property, value);
610-
611-
return (NpgsqlValueGenerationStrategy?)property.SetOrRemoveAnnotation(
612-
NpgsqlAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)
613-
?.Value;
614-
}
604+
=> (NpgsqlValueGenerationStrategy?)property.SetOrRemoveAnnotation(
605+
NpgsqlAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)?.Value;
615606

616607
/// <summary>
617608
/// Sets the <see cref="NpgsqlValueGenerationStrategy" /> to use for the property for a particular table.
@@ -650,11 +641,7 @@ public static void SetValueGenerationStrategy(
650641
public static void SetValueGenerationStrategy(
651642
this IMutableRelationalPropertyOverrides overrides,
652643
NpgsqlValueGenerationStrategy? value)
653-
{
654-
CheckValueGenerationStrategy(overrides.Property, value);
655-
656-
overrides.SetOrRemoveAnnotation(NpgsqlAnnotationNames.ValueGenerationStrategy, value);
657-
}
644+
=> overrides.SetOrRemoveAnnotation(NpgsqlAnnotationNames.ValueGenerationStrategy, value);
658645

659646
/// <summary>
660647
/// Sets the <see cref="NpgsqlValueGenerationStrategy" /> to use for the property for a particular table.
@@ -667,37 +654,8 @@ public static void SetValueGenerationStrategy(
667654
this IConventionRelationalPropertyOverrides overrides,
668655
NpgsqlValueGenerationStrategy? value,
669656
bool fromDataAnnotation = false)
670-
{
671-
CheckValueGenerationStrategy(overrides.Property, value);
672-
673-
return (NpgsqlValueGenerationStrategy?)overrides.SetOrRemoveAnnotation(
674-
NpgsqlAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)
675-
?.Value;
676-
}
677-
678-
private static void CheckValueGenerationStrategy(IReadOnlyProperty property, NpgsqlValueGenerationStrategy? value)
679-
{
680-
if (value is not null)
681-
{
682-
var propertyType = property.ClrType;
683-
684-
if ((value is NpgsqlValueGenerationStrategy.IdentityAlwaysColumn or NpgsqlValueGenerationStrategy.IdentityByDefaultColumn)
685-
&& !IsCompatibleWithValueGeneration(property))
686-
{
687-
throw new ArgumentException(
688-
NpgsqlStrings.IdentityBadType(
689-
property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
690-
}
691-
692-
if (value is NpgsqlValueGenerationStrategy.SerialColumn or NpgsqlValueGenerationStrategy.SequenceHiLo
693-
&& !IsCompatibleWithValueGeneration(property))
694-
{
695-
throw new ArgumentException(
696-
NpgsqlStrings.SequenceBadType(
697-
property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
698-
}
699-
}
700-
}
657+
=> (NpgsqlValueGenerationStrategy?)overrides.SetOrRemoveAnnotation(
658+
NpgsqlAnnotationNames.ValueGenerationStrategy, value, fromDataAnnotation)?.Value;
701659

702660
/// <summary>
703661
/// Returns the <see cref="ConfigurationSource" /> for the <see cref="NpgsqlValueGenerationStrategy" />.

‎src/EFCore.PG/Infrastructure/Internal/NpgsqlModelValidator.cs‎

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,55 @@ protected override void ValidateValueGeneration(
103103
}
104104
}
105105

106+
/// <inheritdoc/>
107+
protected override void ValidateTypeMappings(
108+
IModel model,
109+
IDiagnosticsLogger<DbLoggerCategory.Model.Validation> logger)
110+
{
111+
base.ValidateTypeMappings(model, logger);
112+
113+
foreach (var entityType in model.GetEntityTypes())
114+
{
115+
foreach (var property in entityType.GetFlattenedDeclaredProperties())
116+
{
117+
var strategy = property.GetValueGenerationStrategy();
118+
var propertyType = property.ClrType;
119+
120+
switch (strategy)
121+
{
122+
case NpgsqlValueGenerationStrategy.None:
123+
break;
124+
125+
case NpgsqlValueGenerationStrategy.IdentityByDefaultColumn:
126+
case NpgsqlValueGenerationStrategy.IdentityAlwaysColumn:
127+
if (!NpgsqlPropertyExtensions.IsCompatibleWithValueGeneration(property))
128+
{
129+
throw new InvalidOperationException(
130+
NpgsqlStrings.IdentityBadType(
131+
property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
132+
}
133+
134+
break;
135+
136+
case NpgsqlValueGenerationStrategy.SequenceHiLo:
137+
case NpgsqlValueGenerationStrategy.Sequence:
138+
case NpgsqlValueGenerationStrategy.SerialColumn:
139+
if (!NpgsqlPropertyExtensions.IsCompatibleWithValueGeneration(property))
140+
{
141+
throw new InvalidOperationException(
142+
NpgsqlStrings.SequenceBadType(
143+
property.Name, property.DeclaringType.DisplayName(), propertyType.ShortDisplayName()));
144+
}
145+
146+
break;
147+
148+
default:
149+
throw new UnreachableException();
150+
}
151+
}
152+
}
153+
}
154+
106155
/// <summary>
107156
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
108157
/// the same compatibility standards as public APIs. It may be changed or removed without notice in

‎test/EFCore.PG.Tests/Metadata/NpgsqlMetadataBuilderExtensionsTest.cs‎

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -75,24 +75,6 @@ public void Can_access_property()
7575
a => a.Name.StartsWith(NpgsqlAnnotationNames.Prefix, StringComparison.Ordinal)));
7676
}
7777

78-
[ConditionalFact]
79-
public void Throws_setting_sequence_generation_for_invalid_type()
80-
{
81-
var propertyBuilder = CreateBuilder()
82-
.Entity(typeof(Splot))
83-
.Property(typeof(string), "Name");
84-
85-
Assert.Equal(
86-
NpgsqlStrings.SequenceBadType("Name", nameof(Splot), "string"),
87-
Assert.Throws<ArgumentException>(
88-
() => propertyBuilder.HasValueGenerationStrategy(NpgsqlValueGenerationStrategy.SequenceHiLo)).Message);
89-
90-
Assert.Equal(
91-
NpgsqlStrings.SequenceBadType("Name", nameof(Splot), "string"),
92-
Assert.Throws<ArgumentException>(
93-
() => new PropertyBuilder((IMutableProperty)propertyBuilder.Metadata).UseHiLo()).Message);
94-
}
95-
9678
[ConditionalFact]
9779
public void Can_access_index()
9880
{

0 commit comments

Comments
 (0)