v10 Removal of UmbracoDatabase property from IScope

[p-m-j]

Background

Umbraco v9 attempted to make the Umbraco.Core project less infrastructure aware by creating a new Umbraco.Infrastructure project and moving classes as appropriate to the new assembly.

Unfortunately some classes didn't end up in their ideal home, many of our application services in the Umbraco.Cms.Core namespace had to live in the Infrastructure project as they depend on our UnitOfWork IScope.

IScope in v9 exposes IUmbracoDatabase which Implements NPoco.IDatabase, but the core project has no reference to NPoco so keeping IScope in the Core assembly was difficult without refactoring.

Breaking changes in v10

To solve the issue, IScope at present in the v10/dev branch has lost the UmbracoDatabase property, this enables us to move the stranded application services to their correct assembly.

But this causes problems for any packages or projects which expect to be able to access the database directly from IScope.

Possible solutions

1. Keep the Umbraco.Cms.Core.Scoping interfaces untouched in the infrastructure assembly whilst signposting via obsolete attributes to new interfaces in the infrastructure namepace + Introduce a new ICoreScope interface to the Cms.Core project for those projects which are transaction but not infrastructure aware.
2. Introduce new infrastructure aware interfaces e.g. IDatabaseScopeProvider/IDatabaseScope & document the removal of the UmbracoDatabase property as a breaking change for downstream projects to handle, this shouldn't take a lot of effort to migrate as all that is changing is a namespace + interface name and can be handled project wide with find+replace.

The internal preference is for solution 1, we think this will make migration to v10 easier for downstream packages/projects as the existing interfaces are untouched, however there is a downside as many constructors for services in the Core project will have to have breaking changes introduced to switch from a dependency on IScope to ICoreScope. Whilst this approach introduces many more breaking changes it seems the lessor of two evils as the constructors are generally considered pseudo internal. It is expected that downstream projects will depend only on the abstractions and have the implementations injected by the IOC container. It is unlikely that these constructors are regularly invoked outside of the Umbraco codebase and we should work towards making this expectation align with reality by changing access modifiers to internal where appropriate moving forwards.

A branch has been setup to demonstrate how this would work v10/dev...v10/feature/fix-scope-provider-friendlyness

• Given that IScope implements ICoreScope in the branch these changes to constructors shouldn't be considdered breaking.
• The introduction of new methods to IScopeProvider is breaking to any downstream implementations.

Questions

1. Do you agree that solution 1 is preferable to solution 2?
2. Are there any better approaches that we have not considered?

[KevinJump]

I think solution 1 makes a lot of sense to me. while it might uglify a bit of the internal ,it does let the obsolete till v12 thing stand (assuming that is when it would go if this went into v10?)

Most of the services will be constructed via the IOC container - so i think that shouldn't cause to many issues.

the one that sprang to my mind briefly looking at the branch is the Upgrader class (used for migrations). Changing the execute method to take ICoreScope vs IScope on that would be a breaking change as the current "recommended" code is to construct this one in a notification handler ?

var upgrader = new Upgrader(new TranslationMigrationPlan());
upgrader.Execute(_migrationPlanExecutor, _scopeProvider, _keyValueService);

(i know we now have package migrations so maybe the old-school migrations should be marked as off-limits but this would break anyone using them?)

couple of examples I found on github:

• https://github.com/skttl/umbraco-fulltextsearch8/blob/v9/dev/src/Our.Umbraco.FullTextSearch/Migrations/ExecuteMigrations.cs#L34
• https://github.com/vendrcontrib/vendr-reviews/blob/v2/release/2.0.0/src/Vendr.Contrib.Reviews/Composing/MigrationComponent.cs#L60

Not sure if there is an intermediate step possible with this - I can't think of many other "services" that are constructed as opposed to injected maybe the pattern is wrong for this one ? also not sure what the options for this one are.

[p-m-j]

Changing the execute method to take ICoreScope vs IScope

In the branch referenced above IScope implements ICoreScope so any instances of this occuring should be fine, I think i'm overselling how breaking solution 1 is (massively as the changes don't appear to be breaking at all).

Edit: There are some breaking changes, IScopeProvider would require the CreateCoreScope method so downstream implementations of IScopeProvider will be broken.

[KevinJump]

could there be a Default Interface implimentation ? - or does it need to do something complicated ?

[p-m-j]

I think that should work, the default implementation for CreateCoreScope can just delegate to CreateScope

[KevinJump]

Also randomly, I did find one project that was creating its own IUserService, this would obviously break, but i am not sure if the expectation on something like this is that you can preserve it between versions ?

https://github.com/azdyrski/Umbraco-User-Reporting/blob/v9/azdyrski.Umbraco-User-Reporting/Services/UmbracoUserService.cs

[KevinJump]

Although looking at this - its internal to that project so would still work if IScopeProvider still existed ?

[p-m-j]

This should continue to work just fine as Umbraco.Cms.Core.Scoping.IScopeProvider continues to exist in the Infrastructure project and can be resolved as required.