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.

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

Migration ignores Fluent API configuration via IEntityTypeConfiguration #34674

Closed
AndrasHorinka opened this issue Sep 13, 2024 · 4 comments
Closed
Labels
area-model-building closed-no-further-action The issue is closed and no further action is planned. customer-reported

Comments

@AndrasHorinka
Copy link

AndrasHorinka commented Sep 13, 2024

.NET 8
Microsoft.EntityFrameworkCore 8.0.8
Microsoft.EntityFrameworkCore.Design 8.0.8
Microsoft.EntityFrameworkCore.Tools 8.0.8
Npgsql.EntityFrameworkCore.PostgreSQL 8.0.4

Code First approach.

Entity

public class AuthenticationAttemptEntity
{
	public long Id { get; set; }
	public Guid StateId { get; set; }
	public bool IsValid { get; set; }
	public DateTimeOffset CreatedAt { get; set; }
	public DateTimeOffset ExpiringAt { get; set; }
	public DateTimeOffset ConsumedAt { get; set; }
}

Configuration

internal class AuthenticationAttemptConfiguration : IEntityTypeConfiguration<AuthenticationAttemptEntity>
{
	public void Configure(EntityTypeBuilder<AuthenticationAttemptEntity> builder)
	{
		builder.ToTable("Attempt", "authentication");
		builder.HasKey(a => a.Id);
		builder.HasIndex(a => a.StateId);

		builder.Property(a => a.Id)
			.HasColumnName("id")
			.IsRequired()
			.UseIdentityAlwaysColumn();

		builder.Property(a => a.StateId)
			.HasColumnName("state_id")
			.IsRequired()
			.HasColumnType("uuid")
			.HasDefaultValueSql("uuid_generate_v4()");

		builder.Property(a => a.IsValid)
			.HasColumnName("is_valid")
			// This is a computed column, TRUE only if the expiration date is in the future and the attempt has not been consumed
			.HasComputedColumnSql("expiring_at > now() AND consumed_at IS NULL", stored: true)
			.IsRequired();

		builder.Property(a => a.CreatedAt)
			.HasColumnName("created_at")
			.HasColumnType("timestamptz")
			.HasDefaultValueSql("now() at time zone 'utc'")
			.IsRequired();

		builder.Property(a => a.ExpiringAt)
			.HasColumnName("expiring_at")
			.HasColumnType("timestamptz")
			.HasDefaultValueSql("now() at time zone 'utc' + interval '10 minutes'")
			.IsRequired();

		builder.Property(a => a.ConsumedAt)
			.HasColumnName("consumed_at")
			.HasColumnType("timestamptz")
			.IsRequired(false);
	}
}

Add-Migration resulted in following Migration file:
Missing

  • schema reference
  • default value definitions for each Timestamp and boolean columns
  • Computed column definition
protected override void Up(MigrationBuilder migrationBuilder)
{
	migrationBuilder.CreateTable(
		name: "AuthenticationAttempt",
		columns: table => new
		{
			Id = table.Column<long>(type: "bigint", nullable: false)
				.Annotation("Npgsql:ValueGenerationStrategy", NpgsqlValueGenerationStrategy.IdentityByDefaultColumn),
			StateId = table.Column<Guid>(type: "uuid", nullable: false),
			IsValid = table.Column<bool>(type: "boolean", nullable: false),
			CreatedAt = table.Column<DateTimeOffset>(type: "timestamp with time zone", nullable: false),
			ExpiringAt = table.Column<DateTimeOffset>(type: "timestamp with time zone", nullable: false),
			ConsumedAt = table.Column<DateTimeOffset>(type: "timestamp with time zone", nullable: false)
		},
		constraints: table =>
		{
			table.PrimaryKey("PK_AuthenticationAttempt", x => x.Id);
		});
}

/// <inheritdoc />
protected override void Down(MigrationBuilder migrationBuilder)
{
	migrationBuilder.DropTable(
		name: "AuthenticationAttempt");
}
}

DbContext implementation

public DbSet<AuthenticationAttemptEntity> AuthenticationAttempt { get; set; }

protected new void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfiguration(new AuthenticationAttemptConfiguration());
}

Hence Database creation fails already.
Fixing the migration Up/Down methods does the work only partially. Fixing the migration file can help to mitigate the database creation error, however doesn't solve the Database interaction.

Following method results in error:

public async Task<IActionResult> CreateDummyAuthAttempt()
{
	var dummyAttempt = new AuthenticationAttemptEntity();
	await _context.AuthenticationAttempt.AddAsync(dummyAttempt);
	await _context.SaveChangesAsync();

	return Ok(dummyAttempt);
}

The database logged the following error:
ERROR: relation "Attempt" does not exist at character 13

INSERT INTO "Attempt" 
	("ConsumedAt", "CreatedAt", "ExpiringAt", "IsValid", "StateId")
VALUES ($1, $2, $3, $4, $5)
RETURNING "Id"

Using Data Annotation on the Entity will help with the navigation though.

[Table("AuthenticationAttempt", Schema = "authentication")]
public class AuthenticationAttemptEntity { ... }
@AndrasHorinka
Copy link
Author

In the meantime I have learnt that is_valid definition is incorrect as now() is volatile, thus cannot be utilized in GENERATED ALWAYS AS statements. Nevertheless, this would cause an issue upon executing the manually adjusted migration script (which I did not get to)

@AndriySvyryd
Copy link
Member

AndriySvyryd commented Sep 21, 2024

Please share a small runnable repro project.

@AndriySvyryd
Copy link
Member

Change

protected new void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfiguration(new AuthenticationAttemptConfiguration());
}

to

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
    modelBuilder.ApplyConfiguration(new AuthenticationAttemptConfiguration());
}

@AndriySvyryd AndriySvyryd closed this as not planned Won't fix, can't repro, duplicate, stale Sep 21, 2024
@AndriySvyryd AndriySvyryd added the closed-no-further-action The issue is closed and no further action is planned. label Sep 21, 2024
@AndriySvyryd AndriySvyryd removed their assignment Sep 21, 2024
@AndrasHorinka
Copy link
Author

Though this response is arriving late, but the proposed change from
protected new void OnModelCreating(ModelBuilder modelBuilder)
to
protected override void OnModelCreating(ModelBuilder modelBuilder)

solved.

Thanks for pointing out my mistake.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area-model-building closed-no-further-action The issue is closed and no further action is planned. customer-reported
Projects
None yet
Development

No branches or pull requests

2 participants