OnDisplaying in custom ShapeTableProvider does not populate data anymore in some cases

[Habbni]

Describe the bug

after upgrading from v1.8.3 to the current build, we can no longer use a custom ShapeTableProvider to add missing alternates for the shape ContentsAdminListCreate using following pattern:

builder.Describe("ContentsAdminListCreate")
    .OnDisplaying(async context =>
    {
        var differentiator = (context.Shape as ContentOptionsViewModel)?.SelectedContentType?.EncodeAlternateElement();
        if (differentiator != null)
        {
            context.Shape.Metadata.Alternates.Add(differentiator + "__ContentsAdminListCreate");
        }
    });

This worked perfectly and it still does for almost any other shape, so we suspect this to be unintended.
With the current preview-2.0.0, or running latest main, context.Shape does not contain any of the shape/model data.
In this case, for example CreatableTypes and SelectedContentType remain empty.

It seems to only affect those shapes created using Initialize, like in this case:

Initialize<ContentOptionsViewModel>("ContentsAdminListCreate", m =>

Shapes created without Initialize still get their properties populated, for example this one:

results.Add(Shape("ContentsButtonActions_SummaryAdmin", new ContentItemViewModel(contentItem)).Location("SummaryAdmin", "ActionsMenu:10")

Orchard Core version

2.0.0-preview-18331

To Reproduce

Steps to reproduce the behavior:

1. Take the latest build (it worked only a few builds ago)
2. Create a custom ShapeTableProvider and register it on Startup
3. Override DiscoverAsync, add above Describe method
4. Set a breakpoint within OnDisplaying, go to any List, e.g. the ContentItems and see no shape related data is being filled in

Expected behavior

We would expect to see context.Shape properties to be populated, like it used to do, and like it still does when adding (for example) the following to the ShapeTableProvider:

builder.Describe("ContentsButtonActions_SummaryAdmin")
.OnDisplaying(async context =>{}

[MikeAlhayek]

@Habbni, are you by any chance using caching for your shapes?

We encountered an issue where the initializer (e.g., Initialize<ContentOptionsViewModel>("ContentsAdminListCreate", m =>) was being called every time, even when the object was cached. This was resolved in PR #16644, which ensures the initializer is only called if the shape is not cached.

I believe this change in behavior might be what you're experiencing.

[MikeAlhayek]

@sebastienros I have not tried to reproduce the issue. But just point out I don't see us using cache:

OrchardCore/src/OrchardCore.Modules/OrchardCore.Contents/Drivers/ContentOptionsDisplayDriver.cs

Line 35 in 4f5cd18

Initialize<ContentOptionsViewModel>("ContentsAdminListCreate", m => BuildContentOptionsViewModel(m, model)).Location("Create:10"),

[Habbni]

Hi and thanks for your reply

No caching configured. I actually downloaded a new clone and did a fresh blog recipe setup, to make sure it is reproducable. Just tried disabling the cache module because your reply sounds reasonable, but the issue persists..

[MikeAlhayek]

@sebastienros can you please take a look at this during triage please?

[sebastienros]

Will check. @Habbni can you try at commits
1- 8fd5c88
2- b316211

to confirm this is the change we suspect?

[github-actions]

We triaged this issue and set the milestone according to the priority we think is appropriate (see the docs on how we triage and prioritize issues).

This indicates when the core team may start working on it. However, if you'd like to contribute, we'd warmly welcome you to do that anytime. See our guide on contributions here.

[Habbni]

works with 8fd5c88:

doesnt with b316211 (see screenshot above)

thanks a lot already guys, really wish i was deeper into that code and could contribute more help than this..

[MikeAlhayek]

@sebastienros so that last PR fixed something and broke another.

[sebastienros]

I will debug to understand what is failing, or if the usage described in this issue is that actual problem and another event should have been used.

[sebastienros]

Thanks @Habbni for confirming that quickly, very helpful.

[Habbni]

I will debug to understand what is failing, or if the usage described in this issue is that actual problem and another event should have been used.

I tried the other events as well. But since it was before posting this issue and my brain reached it´s 2bit capacity, I´m not sure if i remember correctly: only OnDisplayed came up with data, which seemed too late for adding an alternate..

[sebastienros]

Correct.

0. Displaying
1. Processing
2. Displayed

I moved the initialization in Processing on the commit you pointed out. I probably should have put it in Displaying since it should not be called on Cache either. Then it will only be a matter of which "Displaying" is called first.

[sebastienros]

• Dynamic Cache uses the DisplayingAsync event to populate the cache, hence these events need to be invoked all the time
• ProcessingAsync was made specifically for code that needs to be executed unless it's cached. Because of that they are executed after DisplayAsync. If you look at MenuShapes it uses ProcessAsync and not DisplayAsync to create the menu nodes specifically for this reason.

What I did for now is invoking the code from Initialize as the first thing done in DisplayAsync to emulate what was done before my PR. I will also move the code from the Notification that is cache to be in ProcessingAsync because we want to run it only when not cached. It may require a new helper on drivers, but I had already suggested it.

Update:

Events are invoked from sources in this order:
IShapeDisplayEvents, applied to any shape, e.g. used by DynamicCache
ShapeDescriptor, e.g. ShapeTableProvider.Describe, applied to specific shape types
ShapeMetadata, e.g. from ShapeResult, aka drivers, applied to specific driver results

So even with my local change, because @Habbni 's code is in a shape table provider, the initialization code is executed too late.

I will need to revert my PR (not technically but revert the change) and update the notification init code to run in Processing.

[MikeAlhayek]

@Habbni sis you get a chance to test out the latest preview? There was a follow up fix after that PR that we thought fixed your issue.

Please try out the latest preview.

Thank you