Feature suggestion: allow callers of IContentManager.BuildDisplay to pass in contextual data for content part drivers to consume

Topics: Core, Customizing Orchard, Writing modules
Developer
Feb 28, 2012 at 3:02 PM
Edited Feb 28, 2012 at 3:03 PM

The situation is that I am invoking the BuildDisplay method from my controller. Some of the drivers that will be invoked require some application specific context, which is available from within the controller.

Currently, I do it like this:

IContent contentPage = GetMeSomeContentPage();
_orchardServices.WorkContext.SetState("MyContextualData", myContextualData);
dynamic pageShape = _contentManager.BuildDisplay(contentPage);

Now, The contentPage has some custom content part attached to it. When its driver is invoked, I want that driver to be able to access the contextual data, which looks like this:

public class CoursePageDriver : ContentPartDriver<CoursePagePart>
    {
        private readonly ICourseService _courseService;
        private readonly IOrchardServices _orchardServices;

        public CoursePageDriver(ICourseService courseService, IOrchardServices orchardServices)
        {
            _courseService = courseService;
            _orchardServices = orchardServices;
        }

        protected override DriverResult Display(CoursePagePart part, string displayType, dynamic shapeHelper)
        {
            var results = new List<DriverResult>
            {
                ContentShape("Parts_CoursePage", () => shapeHelper.Parts_CoursePage(Part: part))
            };

            var chapter = _courseService.GetChapter(part.ChapterId);
            var course = _courseService.GetCourse(chapter.CourseId);
// Here we retrieve the contextual data from the current work context, which was set just before the call to BuildDisplay from the controller:
                var myContextualData = _orchardServices.WorkContext.GetState<CourseSessionRecord>("MyContextualData");
                
// I use the contextual data to generate some shapes that require this data:
                results.Add(ContentShape("SimulationProgress", () => shapeHelper.SimulationProgress( MyContextualData: myContextualData )));
                results.Add(ContentShape("SimulationRetries", () => shapeHelper.SimulationRetries( MyContextualData: myContextualData)));
            }
            
            return Combined(results.ToArray());
        }

    }

So this basically works just fine: I set some global data using SetState<> from the controller, and then fetch that data using GetState<> from within some driver.

What I think would be nice is to be able to pass in some context object into the call to BuildDisplay, e.g.:

_contentManager.BuildDisplay(contentPage, myContextualData);

then I would have access to this context like this:

protected override DriverResult Display(CoursePagePart part, string displayType, dynamic shapeHelper, object context)
{
  var myData = (MyContextualData)context;
... }

Perhaps in a future version the Display method could be refactored into receiving just a single argument with type DisplayArgs<T>, which will hold the content part, display type, shapeHelper, and application specific context.

Although this is probably an edge case, I think it'd be quite handy to be able to pass in application specific data to the drivers when building application specific modules (it's probably not very useful for generic modules and parts).

Feb 28, 2012 at 3:19 PM

Yep, it's something I've commented on before. Since it was something I needed, I actually reimplemented the whole BuildDisplay process in my Origami module, allowing you to pass in all kinds of contextual data. But yeah, I'd like to see it in core in a future Orchard version.

Developer
Feb 28, 2012 at 3:59 PM

Cool. Perhaps it's an idea to create a pull request and create an overloaded version of BuildDisplayManager and enhance the driver base class? Do you have more enhancements like this in mind? Perhaps you could list and propose them to the chieftain.

Feb 28, 2012 at 4:31 PM

The Origami module does a whole load of other things as well so it's not as simple as a pull request. It has its own completely separate display pipeline, which includes a ModelDriver<TModel> so you can create drivers and compose shapes for literally any model, not just content. It just happened that once I'd done all that, it was relatively easy to replace IContentDisplay with a version that used Origami.

Making the change in core starts off relatively easy, just a case of IContentManager.BuildDisplay getting a new signature:

public dynamic BuildDisplay(IContent content, string displayType="Detail", string groupId=null, object data = null);

Data is then available on ContentDisplayContext. The singular problem with this is that driver Display/Editor/Update methods don't actually have access to the context. And you can't change the signature because that would break almost all modules. So you'd have to implement IContentPartDriver instead of ContentPartDriver<T>, and then you lose a whole load of useful stuff (look at the implementation of ContentPartDriver<T> to see what I mean). However this could be improved by separating ContentPartDriver<T>'s functionality into a base class and providing a new version e.g. ContextContentPartDriver<T> that had different overload methods which gave you the context.

I think the real problem is convincing the powers that be that this is useful enough for a change in such a core area :)

There are certainly some other enhancements that could be made in the display code. Particularly when it comes to Placement, and also the stuff I mentioned about building non-content models. This is all stuff I've been experimenting with in Origami and it's incredibly powerful.

Developer
Feb 28, 2012 at 4:37 PM

In situations like this I've set properties of parts before building the display or in a handler to transfer such kind of data (overwriting a default; it's important to have some default here)

The problem with such contexts is that (IMO) Orchard does not, should not work this way: meaning when building a content item's display actually the individual parts should figure out what to do, without external interception. Imagine, any content type can just be displayed from the standard UI when there is no custom context present.

Developer
Feb 28, 2012 at 4:39 PM
randompete wrote:

I think the real problem is convincing the powers that be that this is useful enough for a change in such a core area :)


I'm afraid you're right about that :)
Your Origami module sounds really cool, and am definitely going to check it out as soon as I get a chance!

Developer
Feb 28, 2012 at 4:46 PM
Edited Feb 28, 2012 at 4:48 PM
Piedone wrote:

The problem with such contexts is that (IMO) Orchard does not, should not work this way: meaning when building a content item's display actually the individual parts should figure out what to do, without external interception. Imagine, any content type can just be displayed from the standard UI when there is no custom context present.


Agreed on that.

However, there are certain application specific cases where it's rather useful to be able to pass context to the drivers, of which I know they will always use that context. For example, I'm building an e-learning course module runner, where the administrator can assemble new content types that consist of the CoursePage part. The drivers for the coursepage part will generate shapes that will display the learner's progress. This progress is stored as part of some sort of course session, which needs to be passed to the CoursePage driver.

Another approach could be to simply set some ViewBag item, or like I did with the WorkContext.

But as randompete said, it's questionable how useful an API change would be (especially a breaking one), even more since it's a bad design when you're building loosly coupled systems.

 

Developer
Feb 28, 2012 at 4:58 PM

Turning things a bit upside-down: what about a service that gets the necessary data (that you now store in the context) with the course's id?

I think you want to have a relationship between content items? Or "course session" is really a session? If it is, you could store that data together with the authenticated user (hence not as a session, but kind of), like having a course "session" indexed by the user's id. Since you can access IAuthenticationService in drivers too, there would be no problem loading the appropriate "session".

But if you want to go simple and you know it's unlikely that those special content parts will be ever used without the surrounding custom logic just go with passing around values in parts, what is not particularly nice, but anyway :-).

Coordinator
Feb 28, 2012 at 5:15 PM

I kinda did it to in a recent changeset by adding some custom fields to BuildDisplayContext. Maybe I could refactor it with a Dictionary, so that anything can be passed to it.

Feb 28, 2012 at 5:23 PM

@Piedone, as skywalker says, sometimes the way you render an item depends on the context you're rendering in, stuff that the parts can't possibly determine on their own. The obvious example is DisplayType - that is itself a very simple piece of context data, but sometimes it's just not fine grained enough for everything you might need!

I was also storing properties on Parts as a hackish way to achieve this. The problem is that content manager caches content items. So once you've set that property, anywhere else that uses the content item will also see the property.

Really there's nothing wrong with context - it's a part of our lives, starting first off with HttpContext. The important thing (from a modular perspective) is allowing the right context data to be available in the right place so the Parts can decide what to do with it. Think about it - you're not proscribing any specific behavior, you're just making some data available and it's still up to modular features to actually use it.

@sebastien - sounds great, altho still doesn't get around the issue that the BuildDisplayContext isn't actually available in a ContentPartDriver<T> ...

Developer
Feb 28, 2012 at 5:45 PM

@Pete: absolutely, contexts are necessary but I don't think passing arbitrary data to BuildDisplay() specifically is something I would like to see. DisplayType is one "canonical" context that every user of IContentManager knows about (or the content manager can set a default). But arbitrary data, like a dictionary that can store anything is only fillable with meaningful data if the user is that specific code that knows what the part expects (i.e. the display is built in that specific controller). That's why I think a reversed chain of flow (not passing something to display or parts, but parts should fetch what's necessary, like from a service), just like with IHttpContextAccessor would be better. Now it's questionable if a service for setting and retrieving data essentially between method calls is a good design pattern (smells a bit like the repository pattern), but I would say it depends.

Feb 28, 2012 at 6:31 PM
Edited Feb 28, 2012 at 6:32 PM

Perhaps you've never come up against a situation where you need this. Sometimes you're rendering content items and you want the parts to have access to some data that you know beforehand, but there is no possible way they can retrieve it from a service (and yes, in such situations you have custom parts which are interacting in a specific way with a larger layout or business process). In other words data that is dependent on the wider context, not data that can simply be determined by a content id. If you'd hit this wall, you'd know about it (actually, I think a particular problem we were discussing, where you were trying to get different dependencies to activate or behave differently based on some arbitrary context, was another example of the same thing).

Anyway you seem to be arguing that it's not "the way Orchard does things" ... but if you look at all the other systems you'll see that usually you can pass completely arbitrary data into most of the services of sufficient complexity - e.g. Shapes, Rules, Tokens ...

Developer
Feb 28, 2012 at 7:03 PM

Actually I had an issue exactly like this in the system where I had that context issue too and I solved it with passing it to a part - tell about preaching water :-). To my excuse, the content type I built there was one solely used "on the fly" (nor the type, nor any items persisted, just thrown together with ActivatingFilters) by that one action.

This definitely doesn't seem like the Orchard way for me, but of course I don't want to tell what's the Orchard way; I can imagine it could come handy.

Developer
Feb 28, 2012 at 8:31 PM
Edited Feb 28, 2012 at 8:33 PM
Piedone wrote:

Turning things a bit upside-down: what about a service that gets the necessary data (that you now store in the context) with the course's id?

I think you want to have a relationship between content items? Or "course session" is really a session? If it is, you could store that data together with the authenticated user (hence not as a session, but kind of), like having a course "session" indexed by the user's id. Since you can access IAuthenticationService in drivers too, there would be no problem loading the appropriate "session".

But if you want to go simple and you know it's unlikely that those special content parts will be ever used without the surrounding custom logic just go with passing around values in parts, what is not particularly nice, but anyway :-).

I most certainly agree that it's better to have content parts fetch what's necessary and thus reversing the chain of flow as you call it.
However, in this case, the user is able to run multiple course sessions (FYI  they're not http context sessions; just some records in the database that store the current page which the user left off for a specific course).
The user can later on choose from the UI which course session he wants to continue. Selecting a session will invoke the controller's "continue" action, which receives the ID of the selected session. It's also this "continue" action that will build the content page shape. Now, each course page has at least the "CoursePagePart". While building this shape, the "CoursePagePart" driver needs a handle on the selected course session.

e.g.

        [Themed]
        public ActionResult Continue(Guid sessionId)
        {
            var session = _sessionManager.GetSession(sessionId);
            var token  = session.AccessToken;

            if (token.DetermineIsExpired(_dateTimeService))
                return RedirectToAction("Expired", new {sessionId});

            var course  = _courseService.GetCourse(token.CourseId);
            var currentPageIndex = session.CurrentPageIndex;
            var currentPage  = _courseService.GetContentByIndex(token.CourseId, currentPageIndex);

           // We set the context here:
            _orchardServices.WorkContext.SetState("Elearning.Session", session);

            // We build the content shape here (currentPage has the "CoursePagePart" attached, and its driver will need the context):
            var pageShape = _orchardServices.ContentManager.BuildDisplay(currentPage);

            var model = Shape.Model
            (
                Session: session,
                AccessToken: token,
                Course: course,
                CurrentPageIndex: currentPageIndex,
                CurrentPage: currentPage,
                CurrentPageShape: pageShape,
            );
            
            return View((object)model);
        }

 

Now in this specific case, content items with the "CoursePagePart" will only ever be used via the course session controller, and will never be navigated to directly. So the context will always be there.

Developer
Mar 3, 2012 at 10:11 AM
Piedone wrote:

Turning things a bit upside-down: what about a service that gets the necessary data (that you now store in the context) with the course's id?


Ah, I suppose I could inject the IWorkContextAccessor and retrieve the session ID from there. Then I wouldn't need to transfer state from the controller to the driver.

Mar 13, 2012 at 1:55 AM
Piedone wrote:

Actually I had an issue exactly like this in the system where I had that context issue too and I solved it with passing it to a part - tell about preaching water :-). To my excuse, the content type I built there was one solely used "on the fly" (nor the type, nor any items persisted, just thrown together with ActivatingFilters) by that one action.

This definitely doesn't seem like the Orchard way for me, but of course I don't want to tell what's the Orchard way; I can imagine it could come handy.

I ended up doing exactly this today. I created two properties on my custom Part and set them in my Controller action. Then in Driver.Display I use those values... Seems very impure but at least it works.