-
Notifications
You must be signed in to change notification settings - Fork 33
Alternative implementation of the ditto chain context #207
Conversation
…e chain context being passed down on any inner .As calls
@mattbrailsford glad it fixes the issue. I'm all for it being being more explicit as to what's going on. Sorry if I'm being thick here, just trying to make things clear in my head. So, when I'm using multiple processors be it either when decorating a property or in a multi-processor I'd use the ChainContext rather than the Context? |
@mattbrailsford Sounds like a good solution. I'll give it a test once it's available via MyGet. To echo what @jamiepollock said, would I also need to use the Assuming, of course, that I have a particular context I want to provide. |
@Nicholas-Westby As this is a PR it won't be built on MyGet. You could always clone the repo and build the package locally using the build.cmd. I have a NuGet feed pointing to a local directory for just this kind of thing.
Just make sure you change your package source to your locally configured nuget feed.
…Sent from my iPhone
On 1 Feb 2017, at 23:27, Nicholas-Westby ***@***.***> wrote:
@mattbrailsford Sounds like a good solution. I'll give it a test once it's available via MyGet.
To echo what @jamiepollock said, would I also need to use the chainContext parameter rather than processorContext? That is, on the very first call to As() (before the nested calls to As())?
Assuming, of course, that I have a particular context I want to provide.
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or mute the thread.
|
@jamiepollock @Nicholas-Westby the ChainContext is really just an internal thing. It keeps a cache of processor contexts. You shouldn't need to interact with it yourself. Your processor will still be passed the relevant processor context via it's Context property, so you should continue to use that. The ChainContext just allows us to pass "Chain related contextual information" down the entire chain. |
@mattbrailsford I built this pull request and updated my local and it seemed to work perfectly without any code changes. ✅ 👍 |
@mattbrailsford Are you happy for this to be merged in? or are there still questions around this? |
Merge away :)
…On 30 May 2017 9:41 p.m., "Lee Kelleher" ***@***.***> wrote:
@mattbrailsford <https://github.com/mattbrailsford> Are you happy for
this to be merged in? *or are there still questions around this?*
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#207 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAgLyVe78j5lBZHEmjDtdiCG0_l4Qy0zks5r_H7dgaJpZM4L0UNT>
.
|
# Conflicts: # src/Our.Umbraco.Ditto/Extensions/PublishedContentExtensions.cs
Alternative fix for issue #201
Chain contexts are used to persist the processor context cache across a chain of processors. The curent implementation tries to be clever by maintaining a cache behind the scenes, but this seems to be causing problems. This implementation makes passing of the chain context through the chain more explicit. All processors now have a property of ChainContext and if .As is called inside a processor and you want it to take part in the chain context then you now have to pass the current chain context in
This seems to fix the issues the other implementation had, but forces the passing of the context to be explicit and thus possible for someone to forget. That said, it's sounding like this may be the most logical solution.
Discuss!