Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
9 changes: 8 additions & 1 deletion src/EFCore.Relational/Migrations/HistoryRepository.cs
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,14 @@ protected virtual string MigrationIdColumnName
.FindProperty(nameof(HistoryRow.MigrationId))!
.GetColumnName();

private IModel EnsureModel()
/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
[EntityFrameworkInternal]
protected virtual IModel EnsureModel()
{
if (_model == null)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using System.Text;
using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal;

namespace Microsoft.EntityFrameworkCore.SqlServer.Migrations.Internal;

Expand All @@ -24,6 +25,36 @@ public SqlServerHistoryRepository(HistoryRepositoryDependencies dependencies)
{
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
/// any release. You should only use it directly in your code with extreme caution and knowing that
/// doing so can result in application failures when updating to a new Entity Framework Core release.
/// </summary>
protected override IReadOnlyList<MigrationCommand> GetCreateCommands()
{
// TODO: This is a hack around https://github.com/dotnet/efcore/issues/34991: provider-specific conventions may add
// database-level annotations (e.g. full-text catalogs) to the model, and the default EF logic causes them to be created
// at this point, when the history table is being created. This is too early, and causes the later actual migration to fail.
// So we filter out full-text catalog annotations from AlterDatabaseOperation.
// This follows the same approach as the Npgsql provider (npgsql/efcore.pg#3713).
#pragma warning disable EF1001 // Internal EF Core API usage.
var model = EnsureModel();
#pragma warning restore EF1001 // Internal EF Core API usage.

var operations = Dependencies.ModelDiffer.GetDifferences(null, model.GetRelationalModel());

foreach (var operation in operations)
{
if (operation is AlterDatabaseOperation alterDatabaseOperation)
{
alterDatabaseOperation.RemoveAnnotation(SqlServerAnnotationNames.FullTextCatalogs);
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It would be better to do this in SqlServerMigrationsSqlGenerator.RewriteOperations

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But isn't the point that we should only be filtering these operations out when creating the history repository (hence this type)? If we filter out in the migrations SQL generator we'd also stop creating the full-text catalogs as part of the regular migrations, no?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You are right, doing it there would also be a hack. Perhaps we should just do #34991 instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

FWIW the hack in this PR has been in place for the PG provider for quite a while... That doesn't make it great, but I'm not sure we should spend any more time on this than we have to right now...

}
}

return Dependencies.MigrationsSqlGenerator.Generate(operations, model);
}

/// <summary>
/// This is an internal API that supports the Entity Framework Core infrastructure and not subject to
/// the same compatibility standards as public APIs. It may be changed or removed without notice in
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// The .NET Foundation licenses this file to you under the MIT license.

using Microsoft.Data.SqlClient;
using Microsoft.EntityFrameworkCore.SqlServer.Metadata.Internal;

// ReSharper disable InconsistentNaming
namespace Microsoft.EntityFrameworkCore.Migrations;
Expand All @@ -21,6 +22,25 @@ [ProductVersion] nvarchar(32) NOT NULL,
CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId])
);

""", sql, ignoreLineEndingDifferences: true);
}

[ConditionalFact]
public void GetCreateScript_works_with_full_text_catalog()
{
// Inject a model finalizing convention that adds a full-text catalog to the model, simulating a scenario where
// provider conventions add database-level annotations. Without filtering, the history table creation script would
// include CREATE FULLTEXT CATALOG.
var sql = CreateHistoryRepository(addFullTextCatalogConvention: true).GetCreateScript();

Assert.Equal(
"""
Comment thread
roji marked this conversation as resolved.
CREATE TABLE [__EFMigrationsHistory] (
[MigrationId] nvarchar(150) NOT NULL,
[ProductVersion] nvarchar(32) NOT NULL,
CONSTRAINT [PK___EFMigrationsHistory] PRIMARY KEY ([MigrationId])
);

""", sql, ignoreLineEndingDifferences: true);
}

Expand Down Expand Up @@ -149,17 +169,28 @@ public void GetEndIfScript_works()
""", sql, ignoreLineEndingDifferences: true);
}

private static IHistoryRepository CreateHistoryRepository(string schema = null)
=> new TestDbContext(
private static IHistoryRepository CreateHistoryRepository(
string schema = null,
Action<ModelBuilder> configureModel = null,
bool addFullTextCatalogConvention = false)
{
var serviceProvider = addFullTextCatalogConvention
? SqlServerTestHelpers.Instance.CreateServiceProvider(
new ServiceCollection().AddSingleton<IConventionSetPlugin, FullTextCatalogConventionPlugin>())
: SqlServerTestHelpers.Instance.CreateServiceProvider();

return new TestDbContext(
new DbContextOptionsBuilder()
.UseInternalServiceProvider(SqlServerTestHelpers.Instance.CreateServiceProvider())
.UseInternalServiceProvider(serviceProvider)
.UseSqlServer(
new SqlConnection("Database=DummyDatabase"),
b => b.MigrationsHistoryTable(HistoryRepository.DefaultTableName, schema))
.Options)
.Options,
configureModel)
.GetService<IHistoryRepository>();
}

private class TestDbContext(DbContextOptions options) : DbContext(options)
private class TestDbContext(DbContextOptions options, Action<ModelBuilder> configureModel = null) : DbContext(options)
{
public DbSet<Blog> Blogs { get; set; }

Expand All @@ -169,9 +200,35 @@ public IQueryable<TableFunction> TableFunction()

protected override void OnModelCreating(ModelBuilder modelBuilder)
{
configureModel?.Invoke(modelBuilder);
}
}

/// <summary>
/// A convention plugin that adds a full-text catalog annotation to the model, simulating what a provider convention
/// might do. This allows testing that <see cref="SqlServerHistoryRepository.GetCreateCommands" /> properly filters
/// out the full-text catalog from the history table creation script.
/// </summary>
private class FullTextCatalogConventionPlugin : IConventionSetPlugin
{
public ConventionSet ModifyConventions(ConventionSet conventionSet)
{
conventionSet.ModelFinalizingConventions.Add(new FullTextCatalogAddingConvention());
return conventionSet;
}
}

private class FullTextCatalogAddingConvention : IModelFinalizingConvention
{
#pragma warning disable EF1001 // Internal EF Core API usage.
public void ProcessModelFinalizing(
IConventionModelBuilder modelBuilder,
IConventionContext<IConventionModelBuilder> context)
=> SqlServerFullTextCatalog.AddFullTextCatalog(
(IMutableModel)modelBuilder.Metadata, "TestCatalog", ConfigurationSource.Convention);
#pragma warning restore EF1001 // Internal EF Core API usage.
}

private class Blog
{
public int Id { get; set; }
Expand Down
Loading