Over the past couple of weeks at Umbraco HQ we've been working on the migration of the Forms package to V9 and .NET Core. I've been blogging recently on the topic, where I've shared articles on various aspects of the work needed and changes found in migrating an open-source package. Unsurprisingly though, that didn't cover everything a package developer might encounter, so I thought it was worth an addendum or two on some further topics found in the Forms migration.
Source Code and Branching
Most developers with a package out for V8 are unlikely to be able to simply call it "done" and start a new repository for V9 - and Forms in no exception. Broadly we expect to align the package with the CMS plans for long-term support, and so we'll still be making bug fixes and, for a while, adding features to the V8 version.
I suspect different developers will adopt different approaches here, so whilst your mileage may vary, this is how we've approached it.
Given the needs of ongoing support of V8, the way we've adopted is to keep the code in the same source repository and create a separate branch for V9 - so we'll have v8/dev
and v9/dev
containing the latest code for these two versions respectively.
In order to do the first pass of the migration, we took it one project at a time - there are four within the Forms solution. For each one, we created a new, empty class library project targetting .NET 5 and referencing the latest nightly of the appropriate CMS reference: Umbraco.Cms.Web.Common
, Umbraco.Cms.Web.BackOffice
or Umbraco.Cms.Web.Website
as needed. And then copied over class files a few at a time, and made the necessary changes such that they at least compile.
Mostly this involves adjusting namespaces and making updates to meet the occasional new syntax or approach of .NET Core. Uses of Umbraco's Current
, often in static methods, were converted to services using dependency injection (more on this below). And any hard stuff was punted for later with some commenting out and TODO
s!
Once complete, the original project was deleted from the solution and the new one renamed and moved such that all the file paths for those classes existing in both V8 and V9 versions were the same, and hence merging between the two branches is as simple as it can be. Depending on the changes it may not be easy, but at least things are aligned to support doing so. The intention moving forward is to merge up from V8 to V9, with less expectation of going back the other way. Hence any feature intended for both branches would likely first be built in V8, and merged up to V9. And over time we'd expect that to reduce to just being bug fixes or security patches that where appropriate are merged up, with new features living only in V9.
And then we start working through the TODO
s. At the time of writing these are mostly done, so the known issues that needed to be handled are complete. As I teased last week, the Forms package now at least runs in the back-office. We of course need to now test the package thoroughly, front-end and back-office, and no doubt some further changes will be needed.
Injecting Dependencies
I discussed a few times in my previous blog posts about how the .NET Core paradigm is much more aligned with dependency injection, with using this approach seen as a best practice, pushing you toward the "pit of success" in creating testable, maintainable code. So, as mentioned, we took a few existing classes that made use of service location and converted them into service classes that could be injected into those classes that required them.
In working through the Forms migration though we came across a few nuances to this, mostly via my colleague Bjarke's code reviews(!), that seemed worth commenting on.
Dependencies and Extension Methods
An extension method, being a static method, can't take dependencies injected through the constructor and rather if it requires something to function, that something needs to be passed in as a method parameter. In the Forms code-base we had one of these - called ParsePlaceholders()
and implemented as an extension method on strings - that's used to support the "magic string" functionality of Forms. It takes a string and replaces various placholders that may be in it with details derived from the current HTTP request, the current member, the current published content or dictionary values.
And as you'd imagine, that means a lot of dependencies! Initially I did just that, adding them as additional parameters to the extension method, passing them in from the calling code that will in turn have the dependencies necessary for parsing placholders injected into them via the constructor.
It was clear though in review that this had become a bit of a mess - the calling code now requiring constructor provided dependencies that it didn't need other than to support this extension method, and the method used in many places always with it's long list of arguments. If we ever needed another dependency, there would be a lot of updates to make.
So I'd suggest this is a smell to look out for - and if you find it consider doing what we did and refactor the extension methods to instance methods on to a service class, perhaps with an interface, that's registered with the dependency injection framework and injected to the classes that require this functionality.
In other words, instead of calling var parsedValue = value.ParsePlacholders(...)
we now do var parsedValue = _placholderParsingService(value)
.
Handing Different Lifetimes of Dependencies
Dependencies registered with the framework can have different lifetimes - transient, scoped or singleton. And one of the rules coming out of that is "you can't injected scoped services into singletons".
We had a few cases where we seemingly needed to do that though, for example we had cases where we were looking to access UmbracoHelper and IPublishedContentQuery in this situation.
So whilst this might be a sign that a wider refactoring is in order, it is possible to access these dependencies via a sort of "semi-service location" - a term I've just made up - by accessing the HttpContext and finding the registered service.
In code, this means rather than doing this in a singleton registered class, which isn't allowed:
private readonly UmbracoHelper _umbracoHelper; public DictionaryHelper(UmbracoHelper umbracoHelper) { _umbracoHelper = umbracoHelper; }
We can do this:
private readonly IHttpContextAccessor _httpContextAccessor; public DictionaryHelper(IHttpContextAccessor httpContextAccessor) { _httpContextAccessor = httpContextAccessor; } public void MyMethod() { var umbracoHelper = _httpContextAccessor.HttpContext.RequestServices.GetRequiredService<UmbracoHelper>(); ... }
And just to add, this doesn't preclude unit testing the code, as the service provider can be wired up on a mocked HttpContext like this:
var serviceProviderMock = new Mock<IServiceProvider>(); UmbracoHelper umbracoHelper = CreateFakeUmbracoHelper(); serviceProviderMock .Setup(x => x.GetService(typeof(UmbracoHelper))) .Returns(umbracoHelper);
Umbraco's Extension Methods
Worth noting here too a few minor changes to properties you may be familiar with in V8 that have moved to extension methods:
On IPublishedContent:
.CreatorName
is now.CreatorName()
.WriterName
is now.WriterName()
On PublishedRequest:
.HasPublishedContent
is now.HasPublishedContent()
Trees and Editors
Other than for namespaces, creating custom back-office trees is broadly unchanged so will be familiar in V9. They continue to inherit from TreeController
and require the same attributes as previously. The only other difference I noted was the use of FormCollection
over FormDataCollection
where this is needed in method parameters.
It's a similar story for API controllers supporting custom editors inheriting from UmbracoAuthorizedJsonController
.
The only significant difference I could find here was on how authorization is handled. In V8 we add an attribute to lock down access to users that have permissions for a particular section: [UmbracoApplicationAuthorize(appName: Constants.System.ApplicationAlias)]
.
With V9, instead we add a .NET Core Authorize attribute referencing the name of a policy: [Authorize(Policy = Constants.AuthorizationPolicies.SectionAccessForms)]
. It's another example of where V9 is aligning with .NET Core standards where it can, rather than adding a layer of customisation.
The policy itself is registered at application startup, which we currently have within a composer (still available in V9). It leverages the SectionRequirement made available in core to actually check the user's permission to a section.
public class UmbracoFormsComposer : IComposer { public void Compose(IUmbracoBuilder builder) { ... builder.Services.AddAuthorization(o => CreatePolicies(o, Cms.Core.Constants.Security.BackOfficeAuthenticationType)); } private void CreatePolicies(AuthorizationOptions options, string backOfficeAuthenticationScheme) => options.AddPolicy(Constants.AuthorizationPolicies.SectionAccessForms, policy => { policy.AuthenticationSchemes.Add(backOfficeAuthenticationScheme); policy.Requirements.Add(new SectionRequirement(Cms.Core.Constants.Applications.Forms)); }); }
Miscellaneous Others
Just quickly wrapping up on a few other changes found in migrating the code.
Security
Where accessing the current user previously required:
private readonly IUmbracoContextAccessor _umbracoContextAccessor; public DictionaryHelper(IUmbracoContextAccessor umbracoContextAccessor) { _umbracoContextAccessor = umbracoContextAccessor; } public void MyMethod() { var currentUser = _umbracoContextAccessor.UmbracoContext.Security.CurrentUser; ... }
It's now via:
private readonly IBackOfficeSecurityAccessor _backOfficeSecurityAccessor; public DictionaryHelper(IBackOfficeSecurityAccessor backOfficeSecurityAccessor) { _backOfficeSecurityAccessor = backOfficeSecurityAccessor; } public void MyMethod() { var currentUser = _backOfficeSecurityAccessor.BackOfficeSecurity.CurrentUser; ... }
Cache Refreshing
The method RefreshByPayload()
on DistributedCache
is now RefreshByPayload()
.
Wrapping Up
As mentioned, we're now mostly in a testing phase with Forms, seeing what issues we encounter at runtime and looking to resolve them. We're also considering some refactorings given we have some opportunity for breaking changes. We'll keep these to a minimum though, and really the only one under consideration above and beyond what is necessary is whether to simplify and only support form definitions being stored in the database moving forward. If you've any views on that, I'd welcome your comments.
Comments
Post a Comment