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

Unable to get SQL Server Identity working properly #59

Closed
MarkFl12 opened this issue Mar 10, 2022 · 7 comments
Closed

Unable to get SQL Server Identity working properly #59

MarkFl12 opened this issue Mar 10, 2022 · 7 comments

Comments

@MarkFl12
Copy link

I'm trying to convert an existing Int32 Id property into a strongly typed Id but I'm having trouble getting EF Core 6.0 to do inserts for me.

It's a standard Int32 Id property with SQL Server generating my values for me but no matter what settings I try in my EF config I keep getting errors such as:

The property 'Tag.Id' does not have a value set and no value generator is available for properties of type 'TagId'. Either set a value for the property before adding the entity or configure a value generator for properties of type 'TagId' in 'OnModelCreating'.

I've gone through the posts on the blog and the documentation here and I can't see anything that would help me?

@andrewlock
Copy link
Owner

Yeah, I can't find the documentation for this anywhere, but as far as I know, you can't use database generation with strongly typed ids/value generators unfortunately 🙁

@space-alien
Copy link

space-alien commented Apr 5, 2022

I have found that adding a custom ValueGenerator seems to allow strongly-typed Ids with SQL server.

At least... I'm in the initial stages of testing, and it's looking promising.

Here's what I've got so far:

[StronglyTypedId(
    backingType: StronglyTypedIdBackingType.Long,
    converters: StronglyTypedIdConverter.EfCoreValueConverter)]
public partial struct ProductId { }

Model configuration:

// "Id" would be picked up by convention, but this doesn't hurt.
builder.HasKey(e => e.Id);

// The order of calls here is important: `HasConversion()` must come before `UseIdentityColumn()`.
builder.Property(e => e.Id)

    // Can we do this by convention for all StronglyTypedIds?
    .HasConversion<ProductId.EfCoreValueConverter>()

    // **** SECRET SAUCE!!! ****
    .HasValueGenerator<ProductIdValueGenerator>()

    // At least one of UseIdentityColumn() and ValueGeneratedOnAdd() is required for EF Core
    // to add the SqlServer:Identity annotation.

    // UseIdentityColumn() *might* not be strictly necessary: Now that EF knows it has
    // a numeric primary key type, it seems to add the identity annotation by convention.
    // But I'm not certain, and being explicit can't hurt.
    .UseIdentityColumn()     

    // This might be redundant after the call to UseIdentityColumn(), but it's certainly required without it.
    // Again, being explicit can't hurt.
    .ValueGeneratedOnAdd();

And the ValueGenerator:

internal class ProductIdValueGenerator : ValueGenerator<ProductId>
{
    private long _id;

    public override bool GeneratesTemporaryValues => true;

    public override ProductId Next(EntityEntry entry)
    {
        _id -= 1;
        return new ProductId(_id); 
    }
}

I'll update this as I find out more, but I'd welcome any initial thoughts.

If this works, perhaps source-generating the ValueGenerator would be the final piece of the puzzle.

@arex388
Copy link

arex388 commented May 23, 2022

Also voting for a source-generated ValueGenerator. I tested it out without UseIdentityColumn() and it worked fine. For the ValueGenerator I used this and it worked fine:

internal sealed class EfCoreValueGenerator :
    ValueGenerator<NoteId> {
    public override NoteId Next(
        EntityEntry entry) => new(-1);

    public override bool GeneratesTemporaryValues => true;
}

@space-alien
Copy link

space-alien commented Sep 4, 2022

@arex388 I think you need to return unique values, per my decrementing version, otherwise you will run into an error if you try to add more than one entity of the same type.

@arex388
Copy link

arex388 commented Sep 4, 2022

@space-alien Yes, you're right. I did run into that issue when I was doing a bulk import and all entities had a -1 for the Id... I fixed it by using Random.Shared.Next() * -1. So, the code changed to this:

internal sealed class EfCoreValueGenerator :
    ValueGenerator<NoteId> {
    public override NoteId Next(
        EntityEntry entry) => new(Random.Shared.Next() * -1);

    public override bool GeneratesTemporaryValues => true;
}

I know people frown on using Random, but the value just needs to exist for a fraction of a second until the database generates the real value, so I'm good with that.

@uladz-zubrycki
Copy link

uladz-zubrycki commented May 9, 2023

It's now supported for EF Core 7. One needs to mark their identity-backed property with a ValueGeneratedOnAdd and provide a ValueConverter for it.

The feature was tracked by dotnet/efcore#11597 and is now described at https://learn.microsoft.com/en-us/ef/core/what-is-new/ef-core-7.0/whatsnew#improved-value-generation

Just note that approach with ValueConverterSelector described at https://andrewlock.net/strongly-typed-ids-in-ef-core-using-strongly-typed-entity-ids-to-avoid-primitive-obsession-part-4/ might still be failing, which is discussed at dotnet/efcore#30749.

@andrewlock
Copy link
Owner

As described above, this should be solved (mostly) in EF Core 7 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants