Open dosolkowski-work opened 3 months ago
You can indeed add annotations to the model; this shouldn't involve any changes to the differ, which works in a general way over any annotations. You'd need to make sure these annotations are flown to the migrations, and then pick them up in a customized MigrationsSqlGenerator. For example, any annotation change on an entity type would ultimately result in an AlterTableOperation reaching the MigrationsSqlGenerator; your customized implementation of it would be aware of your custom annotations and generate the correct DDL to account for the changes.
@roji Thanks for the quick response! One question, what do you mean by "make sure these annotations are flown to the migrations"? I think that's the key part I'm missing right now. If I just manually add an annotation to a table, and then run dotnet ef migrations add SomeTest
, there isn't anything in the generated file. If I write a custom CSharpMigrationOperationGenerator
to change how the file is generated, there are no migration operations passed to it. Something upstream of that in EF Core has already decided that the annotation is not something that requires migration--I have a bad feeling that's the MigrationsModelDiffer
that can't be easily and safely customized, so I think that's where I'm stuck?
We configure our tables using classes implementing IEntityTypeConfiguration<Table>
and ApplyConfigurationsFromAssembly()
, which seems to work fine. In the IEntityTypeConfiguration<Table>.Configure(EntityTypeBuilder<Table> builder)
method, I call builder.HasAnnotation("property", "value")
for test purposes, and I see it show up in the context snapshot. However, if I try to customize MigrationsModelDiffer.Diff(ITable source, ITable target, DiffContext diffContext)
(I think that's about here in 8.0) just to experiment, that annotation doesn't show up in either source.GetAnnotations()
or target.GetAnnotations()
, so there won't be an AlterTableOperation
generated. Is there a different way I need to add the annotation?
what do you mean by "make sure these annotations are flown to the migrations"?
Check out e.g. SqlServerAnnotationProvider - if you want model annotations to flow to the migrations SQL generator, they have to be added there. No need to touch the differ.
That would require us to generate SQL code to handle any prior state though, because by that point we don't know what the prior model is, right? I would like to build something that runs when I execute dotnet ef migrations add <Something>
that will let me inject code into the <Timestamp>_<Something>.cs
file that is generated which can then say "grant permissions A and B, revoke permissions X and Y", because it knows what the permissions were before and what they are now. I'm not sure this can be done at any point afterwards.
Additionally, I tried making a custom annotation provider just to experiment and I could not get it to run during dotnet ef migrations add
; if it is tied to the particular database being used, then I assume it doesn't even run until the migrations are actually being turned into SQL?
I was able to create a minimal reproduction: https://github.com/dosolkowski-work/dotnet-ef-migrations-issue
Is there a correct/supported way in EF Core 8 (or planned for 9+) to customize the detection of model changes that should result in a migration, and customize the migration generation itself? We would like to track table permissions as part of our model so they can be specified declaratively and let code decide when and how to grant/revoke, without having to do it all as manual migration operations. For example, when we recently changed up our database service roles a bit, I had to go back through our entire migration history to reconstruct what the permissions should be to apply them to a new role.
My current best guess is that, although I could use annotations to store this custom data in the model, it requires deep diving into
MigrationsModelDiffer
--which is considered internal and unstable--in order to extend EF Core to understand something has actually changed that requires a migration. Is that the only option?