ContentManager.LoadAsync is not called for the content items when listing them on admin

[barthamark]

The List action in OrchardCore.Contents/Controllers/AdminController.cs doesn't initialize content items (i.e. _contentManager.LoadAsync(pageOfContentItems) is missing). Is this intentional? Because of this the content handlers are also not activated.

[Skrypt]

Why would we require to call these handlers when we are simply listing these content items? LoadAsync() is usefull for when you want to do something with the data before displaying it. Here it would mean that you want to alter the content items before displaying them? Generally we do this before editing a content item and then we do a draft or published content item with it. So the data is persisted in the DB. Not sure if we want to alter content items when listing them.

Give me a use case that would require this?

[barthamark]

My goal is not to alter the displayed content items but to provide extensibility to do something in ContentHandlers/ContentPartHandlers. One very simple use case is to weld a part in the Loading event to display something extra or add some extra functionality dynamically. Another example is to add a LazyField (I know that there is no LazyField that we had in Orchard 1 but still we can provide lazy data initialized in Handlers using Lazy<T>).

IContentManager.LoadAsync() is already called inside IContentManager.Get() - so basically when you are displaying the Content Item with the default ItemController it is called too. Displaying them in a list is not different from it, except we are displaying the items with "SummaryAdmin" display type instead of "Detail".

IContentManager.LoadAsync() is also called inside TaxonomyOrchardHelperExtensions.GetInheritedTermsAsync() for example which is basically for fetching taxonomy terms and since it is in an OrchardHelper it is presumably for displaying the fetched items as well.

In Orchard 1 this extensibility was provided automatically when querying items but since in Orchard Core we do it using a lower level service (ISession) we need something that execute the Handler pipeline and LoadAsync does it -- and my assumption (but maybe I am wrong) this is why it has been added in the first place.

[Skrypt]

IContentDisplayHandler BuildDisplayAsync() event.

[barthamark]

What is the problem with LoadAsync being called in the List?

IContentDisplayHandler BuildDisplayAsync() event is good if I want to do such things right before displaying something (I did it actually as a workaround for now) but I wouldn't call it an ultimate solution. What if I want these done in a headless CMS? Or any other case without Display Management?

We have ContentHandlers which is exactly for ContentItem-related event handling and this is what I need. I'd like to do operations with the ContentItem which is already provided when you call IContentManager.Get but not provided when you query Content Items because it is done in a lower level -- except if you call LoadAsync for the list which is an acceptable solution.

[Skrypt]

I need to check if we can do this. But the issue is that we call BuildDisplayAsync event to render the content as a summary in that list. So we build a shape with the content item and return it to the view. That's how the DisplayManager works. Here, I need to understand your point for a headless CMS as the admin UI is all built using the DisplayManager.

Let's then move away from that Content List example and try to think about a solution for a frontend module that would list all content items without using the DisplayManagement. In that case that would make sense to not call the BuildDisplayAsync() method. It would be useless for frontend unless you want to use the same DisplayManagement on your frontend to have the ability to use Content-Summary templates for example.

Then, I need to check for your requirement what would make sense. The issue is that your frontend is headless and it's rendering something different on your backend right?

[Skrypt]

Ok so one part of the issue is that you require this :

        public async Task<ContentItem> LoadAsync(ContentItem contentItem)
        {
            if (!_contentManagerSession.RecallVersionId(contentItem.Id, out var loaded))
            {
                // store in session prior to loading to avoid some problems with simple circular dependencies
                _contentManagerSession.Store(contentItem);

                // create a context with a new instance to load
                var context = new LoadContentContext(contentItem);

                // invoke handlers to acquire state, or at least establish lazy loading callbacks
                await Handlers.InvokeAsync((handler, context) => handler.LoadingAsync(context), context, _logger);
                await ReversedHandlers.InvokeAsync((handler, context) => handler.LoadedAsync(context), context, _logger);

                loaded = context.ContentItem;
            }

            return loaded;
        }

[Skrypt]

I'm trying this in AdminController.cs of OC.Content module and will need to test perf impact. But as suggested by @barthamark it would make sense to have this to call the LoadingAsync and LoadedAsync events on each content item of that list.

            var pagerShape = (await New.Pager(pager)).TotalItemCount(maxPagedCount > 0 ? maxPagedCount : await query.CountAsync()).RouteData(routeData);
            var pageOfContentItems = await query.Skip(pager.GetStartIndex()).Take(pager.PageSize).ListAsync();
            pageOfContentItems = await _contentManager.LoadAsync(pageOfContentItems);

            //We prepare the content items SummaryAdmin shape
            var contentItemSummaries = new List<dynamic>();
            foreach (var contentItem in pageOfContentItems)
            {
                contentItemSummaries.Add(await _contentItemDisplayManager.BuildDisplayAsync(contentItem, _updateModelAccessor.ModelUpdater, "SummaryAdmin"));
            }

[Skrypt]

@barthamark Please provide a PR with explanations. I agree with the suggestion.

[Skrypt]

If we can find a better way to handle this that would also be nice. Because here everytime we do a session.Query and that we use the data somewhere it would require that we also call the handlers manually each time.

[barthamark]

Sounds good, thank you. You are right, a better approach would be nice (maybe implementing an IContentManager.Query that proxies ISession.Query and then calls the LoadAsync?).

A little bit more detailed explanation about my issue(s) about this:

My current issue is that on admin list the ContentHandler.Loading event is not called (or any ContentHandler event) and I can't initialize a Lazy<T> on one of the ContentParts that I am developing and your suggestion about IContentDisplayHandler BuildDisplayAsync() is a possible solution but not for every use case.

Headless CMS was just an example where my goal was to explain this issue in a more generic way so we can figure out a solution which resolves my issue and any future issues related to this.

So let's move away from the Content List example:

When querying ContentItems the ContentHandlers are not used at all. But when we are using IContentManager to Get one particular item then ContentHandlers are used, and this is what I expect. This is what happened in Orchard 1 as well and this way you could do Content Item-level event handling such as welding a part from a handler or initializing a LazyField. Except in Orchard 1 we could do it after querying ContentItems as well. And this is unrelated from Display Management or any other different abstraction level.

So how can we activate ContentHandlers after a query? Until now I used IContentManager.LoadAsync for running ContentHandler events after querying items with ISession.Query which is acceptable and used this in my custom services and controllers.

A couple more examples for what I'd do in Handlers (and again, though I would benefit from it when displaying the content item it doesn't mean that this is the only way I'd benefit from it so it's not display management-related):

• Weld a Part to a Content Item automatically right after getting them from the database. E.g. mark the item for some reason.
• Initialize a Lazy<T> on a ContentPart.
  • E.g. Lazy<IEnumerable<ContentItem>> SimilarContentItems and in the Handler we can initialize this "LazyField" to query similar items.
  • Another example Lazy<DateTime> StartDateLocal where the LocalStartDate is the local time of the StartDateUtc stored on the part and I'd use this from the code a lot of times from in a lot of different services where I don't want to repetitively just localize the date when I can make a LazyField for it so it is done in one place, in the Handler (like we could do it in Orchard 1 and utilized it a hundred times).
• Creating a log entry when a specific content item was fetched from the database. (Weird example but I think you got the point.)

[sebastienros]

It looks like a valid concern, that anything that creates a query to load content items without passing though the CM might be an issue as the Loading/Loaded event would not be called. It should not impact performance either. The idea is that we should reuse existing content items in memory (IdentityMap) at least.

[sebastienros]

And looking at the Load method, it really triggers the events the first time it is called.

[Piedone]

How about running LoadAsync() every time a content item is accessed via the ContentManager? I'd actually make it private too, there shouldn't be a scenario where people have to call it by hand. And for this, I'd create something like IContentManger.Query() that starts a specially prepared YesSQL query to include this functionality (otherwise it wouldn't be any other query feature, it would just let you access ISession).

The point is that let's manage everything necessary for content items to work under the hood, not make the consumer know how to do it. If somebody wants to query the documents directly then they can do that but that shouldn't be the default approach.