Fix custom dbcontexts extention methods

[nikcio]

Prerequisites

• I have added steps to test this contribution in the description below

If there's an existing issue for this PR then this fixes #14893

Description

Why

This problem described in #14893 stems from the database provider not being registered when adding a custom DbContext. The reason the problem was introduced in #14674 was because it was unclear what the extension methods usage was. I.E you couldn't easily see that you were supposed to use one method internally (In Umbraco source code) and one from a consuming application.

Therefore, I think that the extension methods needs to be unified and follow the same basic behavioral patterns as the original AddDbContext() methods from EF Core. In this way I also think it would be easier for experienced EF Core developers to start using it in an Umbraco context.

To solve this problem this PR adds two new extensions:

AddUmbracoDbContext<T>(this IServiceCollection services, Action<DbContextOptionsBuilder>? optionsAction = null)

Umbraco equivalent to EF Cores AddDbContext(this IServiceCollection, Action<DbContextOptionsBuilder>? optionsAction = null, ServiceLifetime contextLifetime = ServiceLifetime.Scoped, ServiceLifetime optionsLifetime = ServiceLifetime.Scoped)

AddUmbracoDbContext<T>(this IServiceCollection services, Action<IServiceProvider, DbContextOptionsBuilder>? optionsAction = null)

Umbraco equivalent to EF Cores AddDbContext(this IServiceCollection, Action<IServiceProvider, DbContextOptionsBuilder>? optionsAction = null, ServiceLifetime contextLifetime = ServiceLifetime.Scoped, ServiceLifetime optionsLifetime = ServiceLifetime.Scoped)

Note:

• Service lifetimes isn't relevant when using AddUmbracoDbContext because we use pooled DbContexts after RuntimeLevel.Run and transient for every DI request before that.
• AddUmbracoDbcontext better matches the EF Core naming of the extension methods.
• Because we don't use a custom Action it's easier to tell what parameters do what. And if you have used EF Core before you will be able to use similar setup in the options.

What is the behavior of AddUmbracoDbcontext()?

Similarly to the normal AddDbContext from EF Core this shouldn't magically add the database provider but you should specify what provider to use. To make this behavior similar to what we had in 12.1 I've added two extension methods that can be used in the optionsAction.

Examples:

builder.Services.AddUmbracoDbContext<CustomDbContext>((serviceProvider, options) =>
{
    options.UseUmbracoDatabaseProvider(serviceProvider);
});

Uses the Umbraco database connection string

builder.Services.AddUmbracoDbContext<CustomDbContext>((options) =>
{
    options.UseDatabaseProvider("My provider name", "Connection string");
});

Uses the provider name and connection string provided and automatically adds the correct provider based on the provider name.

builder.Services.AddUmbracoDbContext<CustomDbContext>((serviceProvider, options) =>
{
    options.UseSqlite("Data Source=:memory:;Version=3;New=True;");
});

Uses the custom provided provider like AddDbContext would. This would also make it possible to add options database specific options to the provider.

What changed

• This PR includes commit 170ed3a by Bjarke Berg (The first one) which is cherry picked from v12/dev. (This adds the UmbracoPooledDbContextFactory)
• Obsolete the old extension methods
• Added some integration tests to test the registration
• Added new Extension methods to replace the old ones
  • ÀddUmbracoDbcontext
  • UseDatabaseProvider
  • UseUmbracoDatabaseProvider
• Added constant values for the database providers.

What doesn't the PR solve

Right now this PR doesn't auto register the database provider on the extension methods on the old AddUmbracoEFCoreContext as this would actually be a breaking change from v12.2 to a version with this change.

But it is quite easy to add we just need to call UseDatabaseProvider on the options in the AddUmbracoEFCoreContext extension methods.

I'm a bit unsure what the best approach is here.

/CC @Zeegaan

Final notes

I think streamlining these extension methods will make it easier to work with EF Core in Umbraco and make a better fit for Umbraco.

Principles of the extension methods:

• A developer should manually specify their database provider like when normally using EF Core but we can provide short hands for using the Umbraco connection string and auto selecting a provider.
• We should support the DbContext itself setting the provider like on the UmbracoDbContext.
• We should allow the developer to specify their own database provider so they can parse what ever options they need and they can use the right database for the them.
• The naming should match EF Core so developers don't need to learn new naming
  • And Options should be set in a similarly to EF Core.

I hope it makes sense

Test

To test this PR I've added some Unit tests. We could also test other combinations of setting the options in similarly.

[github-actions]

Hi there @nikcio, thank you for this contribution! 👍

While we wait for one of the Core Collaborators team to have a look at your work, we wanted to let you know about that we have a checklist for some of the things we will consider during review:

• It's clear what problem this is solving, there's a connected issue or a description of what the changes do and how to test them
• The automated tests all pass (see "Checks" tab on this PR)
• The level of security for this contribution is the same or improved
• The level of performance for this contribution is the same or improved
• Avoids creating breaking changes; note that behavioral changes might also be perceived as breaking
• If this is a new feature, Umbraco HQ provided guidance on the implementation beforehand
• 💡 The contribution looks original and the contributor is presumably allowed to share it

Don't worry if you got something wrong. We like to think of a pull request as the start of a conversation, we're happy to provide guidance on improving your contribution.

If you realize that you might want to make some changes then you can do that by adding new commits to the branch you created for this work and pushing new commits. They should then automatically show up as updates to this pull request.

Thanks, from your friendly Umbraco GitHub bot 🤖 🙂

[emmagarland]

Hi @nikcio

Thanks for your PR to fix #14893, where EF Migrations are broken from v12, and for the added tests too!

One of the Core Collaborators team will review this as soon as possible - it also seems like one that we want HQ to review, so I'll contact them about it.

Best wishes

Emma

[bergmania]

Added a few comments :)

[nikcio]

PR updated 😄
But what should we do about what I described in "What doesn't the PR solve"?

[nikcio]

@bergmania let me know if you're waiting for me to do something else on this PR ☺️

[Zeegaan]

@nikcio I think about what the "PR doesn't solve" is okay, as it is obsoleted,
We could even put some info in the obsolete method about it not working properly to warn 🤷
If you could fix the merge conflict I will get this merged 🚀

[nikcio]

@Zeegaan I've now rebased the PR 😄

[Zeegaan]

Thanks! I will start testing 🚀

[Zeegaan]

Looks good, tests good 🚀

[bjarnef]

@Zeegaan would it be better to move the constants class to "Umbraco.Core" project, where we have most constants?
https://github.com/umbraco/Umbraco-CMS/tree/contrib/src/Umbraco.Core

Then it can be used without reference to Umbraco.Cms.Persistence.EFCore and developers can reference these just with a dependency to Umbraco.Core.

[Zeegaan]

It can yes, good point 👍

[jemayn]

I've found this after starting a new site on 12.3 using EF Core and finding the obsolete mention on the old AddUmbracoEFCoreContext method I've used on other sites.

I have looked at the new extension method and I really like the UseUmbracoDatabaseProvider method compared to the complexity of the old methods.

However, I have a specific use case that I could solve before that I can't spot how to do with the new extension methods:

If I am running a site where I keep migrations in a seperate project to the Umbraco website project, I could before generate the migrations in that project and register the DI there as long as the project had Umbraco.Cms.Persistence.EFCore installed.

However, to make the migrations run I had to specify that the migrations lived in a seperate assembly to the website like this:

In the project with the migrations:

public static class DependencyInjection
{
    public static IServiceCollection AddPersistence(this IServiceCollection services)
    {
        services.AddUmbracoEFCoreContext<FavouriteListContext>((options, connectionString, providerName) => options.UseSqlServer(connectionString,
            builder =>
            {
                builder.MigrationsAssembly(typeof(DependencyInjection).Assembly.GetName().FullName);
            }));

        return services;
    } 
}

And then in the websites startup file:

services.AddUmbraco(_env, _config)
            .AddBackOffice()
            .AddWebsite()
            .AddComposers()
            .Build();
        
services.AddPersistence();

Now with the new methods I can get the connection string for the Umbraco db automatically but not set the assembly:

services.AddUmbracoDbContext<MovieContext>((serviceProvider, options) =>
{
    options.UseUmbracoDatabaseProvider(serviceProvider);
});

Is there a way to both set the database to be the same, and also still configure the assembly with the new extension methods?

[nikcio]

@jemayn For a quick fix that closely matches what you had with the previous extension method you could use:

services.AddUmbracoDbContext<MovieContext>((serviceProvider, options) =>
{
    ConnectionStrings connectionStrings = serviceProvider.GetRequiredService<IOptionsMonitor<ConnectionStrings>>().CurrentValue;

    // Replace data directory
    string? dataDirectory = AppDomain.CurrentDomain.GetData(Constants.System.DataDirectoryName)?.ToString();
    if (string.IsNullOrEmpty(dataDirectory) is false)
    {
        connectionStrings.ConnectionString = connectionStrings.ConnectionString?.Replace(Constants.System.DataDirectoryPlaceholder, dataDirectory);
    }

    if (string.IsNullOrEmpty(connectionStrings.ProviderName))
    {
        Log.Warning("No database provider was set. ProviderName is null");
        return;
    }

    if (string.IsNullOrEmpty(connectionStrings.ConnectionString))
    {
        Log.Warning("No database provider was set. Connection string is null");
        return;
    }

    builder.UseSqlServer(connectionString, x => 
    {
        x.MigrationsAssembly(typeof(MovieContext).Assembly.FullName);
    });
});

We could make this easier by taking extracting the getting the Umbraco connection string to it's own extension method so that UseUmbracoDatabaseProvider would just call builder.GetUmbracoDatabaseConnection (or whatever naming makes sense) and builder.UseDatabaseProvider.

Then with a new extension method the new example would be:

services.AddUmbracoDbContext<MovieContext>((serviceProvider, options) =>
{
    ConnectionStrings connectionStrings = builder.GetUmbracoDatabaseConnection(serviceProvider);

    builder.UseSqlServer(connectionString, x => 
    {
        x.MigrationsAssembly(typeof(MovieContext).Assembly.FullName);
    });
});

[nikcio]

@jemayn I've opened a PR here: #15186 to make the above example easier 😄

[umbrabot]

Hi there @nikcio!

First of all: A big #H5YR for making an Umbraco related contribution during Hacktoberfest! We are very thankful for the huge amount of PRs submitted, and all the amazing work you've been doing 🥇

Due to the amazing work you and others in the community have been doing, we've had a bit of a hard time keeping up. 😅 While all of the PRs for Hacktoberfest might not have been merged yet, you still qualify for receiving some Umbraco swag, congratulations! 🎉

In the spirit of Hacktoberfest we've prepared some exclusive Umbraco swag for all our contributors - including you!

As an alternative choice this year, you can opt-out of receiving anything and ask us to help improve the planet instead by planting a tree on your behalf. 🌳

Receive your swag or plant a tree! 👈 Please follow this link to fill out and submit the form, before December 15th, 2022, 23:59:00 UTC.

Following this date we'll be sending out all the swag, but please note that it might not reach your doorstep for a few months, so please bear with us and be patient 🙏

📢 We have just published a summary of all the activity throughout this year's Umbraco Hacktoberfest.

The only thing left to say is thank you so much for participating in Hacktoberfest! We really appreciate the help!

Kind regards,
The various Umbraco teams.