Skip to content

Fix for issue #151 #179

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Fix for issue #151 #179

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
92 changes: 92 additions & 0 deletions EFCore.CheckConstraints.Test/ValidationCheckConstraintTest.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -192,13 +192,73 @@ public virtual void Properties_on_complex_type()
Assert.Equal("[Location_Latitude] BETWEEN -90.0E0 AND 90.0E0", latitudeCheckConstraint.Sql);
}

[Fact]
public void RequiredWithMaxStringLength()
{
var entityType = BuildEntityType<Blog>();

var checkConstraint = Assert.Single(entityType.GetCheckConstraints(), c => c.Name == "CK_Blog_RequiredWithMaxStringLength_MinLength");
Assert.NotNull(checkConstraint);
Assert.Equal("LEN([RequiredWithMaxStringLength]) >= 1", checkConstraint.Sql);
}

[Fact]
public void RequiredWithMinAndMaxStringLength()
{
var entityType = BuildEntityType<Blog>();

var checkConstraint = Assert.Single(entityType.GetCheckConstraints(), c => c.Name == "CK_Blog_RequiredWithMinAndMaxStringLength_MinLength");
Assert.NotNull(checkConstraint);
Assert.Equal("LEN([RequiredWithMinAndMaxStringLength]) >= 3", checkConstraint.Sql);
}

[Fact]
public void RequiredWithMinLength()
{
var entityType = BuildEntityType<Blog>();

var checkConstraint = Assert.Single(entityType.GetCheckConstraints(), c => c.Name == "CK_Blog_RequiredWithMinLength_MinLength");
Assert.NotNull(checkConstraint);
Assert.Equal("LEN([RequiredWithMinLength]) >= 3", checkConstraint.Sql);
}

[Fact]
public void RequiredWithLength()
{
var entityType = BuildEntityType<Blog>();

var checkConstraint = Assert.Single(entityType.GetCheckConstraints(), c => c.Name == "CK_Blog_RequiredWithLength_MinMaxLength");
Assert.NotNull(checkConstraint);
Assert.Equal("LEN([RequiredWithLength]) BETWEEN 1 AND 4", checkConstraint.Sql);
}

[Fact]
public void RequiredWithLengthAndStringLengthAndMinLength()
{
var entityType = BuildEntityType<Blog>();

var checkConstraint = Assert.Single(entityType.GetCheckConstraints(), c => c.Name == "CK_Blog_RequiredWithLengthAndStringLengthAndMinLength_MinMaxLength");
Assert.NotNull(checkConstraint);
Assert.Equal("LEN([RequiredWithLengthAndStringLengthAndMinLength]) BETWEEN 4 AND 10", checkConstraint.Sql);
}

[Fact]
public void ShouldThrowWhenDeterminedMaxLengthIsGreaterThanMinLength()
{
var exception = Assert.Throws<InvalidOperationException>(() => BuildEntityType<MinLengthExceedingMaxLength>());
Assert.Equal(
$"The minimum length (400) specified for [{nameof(MinLengthExceedingMaxLength)}].[{nameof(MinLengthExceedingMaxLength.Property)}] exceeds the maximum allowable length (10).",
exception.Message);
}

#region Support

// ReSharper disable UnusedMember.Local
class Blog
{
public int Id { get; set; }

[Required]
[Range(1, 5)]
public int Rating { get; set; }

Expand Down Expand Up @@ -245,7 +305,39 @@ class Blog
public required string StartsWithA { get; set; }

public required Location Location { get; set; }

[Required]
[StringLength(100)]
public required string RequiredWithMaxStringLength { get; set; }

[Required]
[StringLength(100, MinimumLength = 3)]
public required string RequiredWithMinAndMaxStringLength { get; set; }

[Required]
[MinLength(3)]
public required string RequiredWithMinLength { get; set; }

[Required]
[Length(0, 4)]
public required string RequiredWithLength { get; set; }

[Required]
[Length(0, 10)]
[StringLength(100, MinimumLength = 3)]
[MinLength(4)]
public required string RequiredWithLengthAndStringLengthAndMinLength { get; set; }
}

class MinLengthExceedingMaxLength
{
[Required]
[Length(0, 10)]
[StringLength(100, MinimumLength = 3)]
[MinLength(400)]
public required string Property { get; set; }
}

// ReSharper restore UnusedMember.Local

[ComplexType]
Expand Down
36 changes: 31 additions & 5 deletions EFCore.CheckConstraints/Internal/ValidationCheckConstraintConvention.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,6 +80,9 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
continue;
}

int? minLength = null;
int? maxLength = null;

foreach (var attribute in memberInfo.GetCustomAttributes())
{
switch (attribute)
Expand All @@ -89,22 +92,24 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
continue;

case MinLengthAttribute a when _intTypeMapping is not null:
AddMinimumLengthConstraint(property, memberInfo, tableName, columnName, sql, a.Length);
minLength = minLength is null ? a.Length : Math.Max(a.Length, minLength.Value);
continue;

case StringLengthAttribute a when _intTypeMapping is not null:
AddMinimumLengthConstraint(property, memberInfo, tableName, columnName, sql, a.MinimumLength);
minLength = minLength is null ? a.MinimumLength : Math.Max(a.MinimumLength, minLength.Value);
continue;

case RequiredAttribute { AllowEmptyStrings: false } when _intTypeMapping is not null:
AddMinimumLengthConstraint(property, memberInfo, tableName, columnName, sql, minLength: 1);
case RequiredAttribute { AllowEmptyStrings: false }
when _intTypeMapping is not null && memberInfo.GetMemberType() == typeof(string):
minLength = minLength is null ? 1 : Math.Max(1, minLength.Value);
continue;

case LengthAttribute a when _intTypeMapping is not null:
// Note: The max length should be enforced by the column schema definition in EF,
// see https://github.com/dotnet/efcore/issues/30754. While that isn't done, we enforce it via the check
// constraint.
AddStringLengthConstraint(property, memberInfo, tableName, columnName, sql, a.MinimumLength, a.MaximumLength);
minLength = minLength is null ? a.MinimumLength : Math.Max(a.MinimumLength, minLength.Value);
Comment on lines +95 to +111
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider renaming 'minLength' to 'computedMinLength' to better reflect that it aggregates values from multiple attributes, which could improve code clarity for future maintainers.

Copilot uses AI. Check for mistakes.
maxLength = maxLength is null ? a.MaximumLength : Math.Min(a.MaximumLength, maxLength.Value);
Copy link

Copilot AI Jun 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] Consider renaming 'maxLength' to 'computedMaxLength' to clearly convey its role in aggregating maximum length limits from multiple attributes.

Suggested change
maxLength = maxLength is null ? a.MaximumLength : Math.Min(a.MaximumLength, maxLength.Value);
computedMaxLength = computedMaxLength is null ? a.MaximumLength : Math.Min(a.MaximumLength, computedMaxLength.Value);

Copilot uses AI. Check for mistakes.
continue;

case AllowedValuesAttribute a:
Expand Down Expand Up @@ -152,6 +157,27 @@ public virtual void ProcessModelFinalizing(IConventionModelBuilder modelBuilder,
}
}
}

if (minLength is null)
{
continue;
}

if (maxLength is not null)
{
if (minLength.Value > maxLength.Value)
{
throw new InvalidOperationException(
$"The minimum length ({minLength}) specified for [{tableName}].[{columnName}] exceeds the maximum allowable length ({maxLength})."
);
}

AddStringLengthConstraint(property, memberInfo, tableName, columnName, sql, minLength.Value, maxLength.Value);
}
else
{
AddMinimumLengthConstraint(property, memberInfo, tableName, columnName, sql, minLength.Value);
}
}
}
}
Expand Down