Notes and cleanup

This commit is contained in:
Shannon
2020-12-11 15:29:27 +11:00
parent b5e3bc9e0d
commit 776df77dfe
7 changed files with 10 additions and 16 deletions

View File

@@ -7,11 +7,6 @@ namespace Umbraco.Core
/// </summary>
public static class Web
{
// TODO: Need to review these...
//public const string UmbracoContextDataToken = "umbraco-context";
//public const string UmbracoDataToken = "umbraco";
//public const string PublishedDocumentRequestDataToken = "umbraco-doc-request";
//public const string CustomRouteDataToken = "umbraco-custom-route";
public const string UmbracoRouteDefinitionDataToken = "umbraco-route-def";
/// <summary>

View File

@@ -49,11 +49,12 @@ namespace Umbraco.Web
/// <summary>
/// Boolean value indicating whether the current request is a front-end umbraco request
/// </summary>
bool IsFrontEndUmbracoRequest { get; }
bool IsFrontEndUmbracoRequest { get; } // TODO: This could easily be an ext method and mocking just means setting the published request to null
/// <summary>
/// Gets/sets the PublishedRequest object
/// </summary>
// TODO: Can we refactor this and not expose this mutable object here? Instead just expose IPublishedContent? A bunch of stuff would need to change but would be better
IPublishedRequest PublishedRequest { get; set; }
/// <summary>

View File

@@ -772,11 +772,10 @@ namespace Umbraco.Core.Persistence.Repositories.Implement
* level now since we can fire these events within the transaction...
* The reason these events 'need' to fire in the transaction is to ensure data consistency with Nucache (currently
* the only thing that uses them). For example, if the transaction succeeds and NuCache listened to ContentService.Saved
* and then NuCache failed at persisting data after the trans completed, then NuCache would be out of sync. This way
* the entire trans is rolled back if NuCache files. That said, I'm unsure this is really required because there
* are other systems that rely on the "ed" (i.e. Saved) events like Examine which would be inconsistent if it failed
* too. I'm just not sure this is totally necessary especially.
* So these events can be moved to the service level. However, see the notes below, it seems the only event we
* and then NuCache failed at persisting data after the trans completed, then NuCache would be out of sync. This way
* the entire trans is rolled back if NuCache files. This is part of the discussion about removing the static events,
* possibly there's 3 levels of eventing, "ing", "scoped" (in trans) and "ed" (after trans).
* These particular events can be moved to the service level. However, see the notes below, it seems the only event we
* really need is the ScopedEntityRefresh. The only tricky part with moving that to the service level is that the
* handlers of that event will need to deal with the data a little differently because it seems that the
* "Published" flag on the content item matters and this event is raised before that flag is switched. Weird.

View File

@@ -31,7 +31,7 @@ namespace Umbraco.Core.Services.Implement
/// The purpose of this event being raised within the transaction is so that listeners can perform database
/// operations from within the same transaction and guarantee data consistency so that if anything goes wrong
/// the entire transaction can be rolled back. This is used by Nucache.
/// TODO: See remarks in ContentRepositoryBase about these types of events. Not sure we need/want them.
/// TODO: See remarks in ContentRepositoryBase about these types of events.
/// </remarks>
public static event TypedEventHandler<TService, ContentTypeChange<TItem>.EventArgs> ScopedRefreshedEntity;

View File

@@ -42,10 +42,6 @@ namespace Umbraco.Tests.Integration.Umbraco.Infrastructure.Services
[SetUp]
public void SetupTestData()
{
// This is super nasty, but this lets us initialize the cache while it is empty.
// var publishedSnapshotService = GetRequiredService<IPublishedSnapshotService>() as PublishedSnapshotService;
// publishedSnapshotService?.OnApplicationInit(null, EventArgs.Empty);
if (_langFr == null && _langEs == null)
{
var globalSettings = new GlobalSettings();

View File

@@ -183,6 +183,8 @@ namespace Umbraco.Extensions
/// </summary>
public static IApplicationBuilder UseUmbracoContentCache(this IApplicationBuilder app)
{
// TODO: This should install middleware to initialize instead of eagerly doing the initialize here
PublishedSnapshotServiceEventHandler publishedContentEvents = app.ApplicationServices.GetRequiredService<PublishedSnapshotServiceEventHandler>();
publishedContentEvents.Start();
return app;

View File

@@ -32,6 +32,7 @@ namespace Umbraco.Web.Website.Routing
/// It seems as though with the "State" parameter we could more easily assign the IPublishedRequest or IPublishedContent
/// or UmbracoContext more easily that way. In the meantime we will rely on assigning the IPublishedRequest to the
/// route values along with the IPublishedContent to the umbraco context
/// have created a GH discussion here https://github.com/dotnet/aspnetcore/discussions/28562 we'll see if anyone responds
/// </remarks>
public class UmbracoRouteValueTransformer : DynamicRouteValueTransformer
{