Bugfixes for MNTP / EntityController

[p-m-j]

Closes #11631 & #11448.

Probably also closes #11647

If you follow manual reproducing steps from #11631

e.g.

1. Create a partial view macro with MNTP property
2. Create doctype with grid
3. Create page with macro in grid -- makes request GetUrlsByIds(int[])

--

If you also add a regular MNTP to the doc type - makes request GetUrlsByIds(Udi[])

Populate all the pickers with content, then you have covered testing for both issues at onces.

---

[bjarnef]

@p-m-j I wonder if the issues I was facing with MNTP is in core as I didn't experienced these errors with MNTP in initial v9.0.0 release. I searched for changed in v9.0.1, but I couldn't find specifically something regarding MNTP and JSON changes.
However Umbraco.Deploy.Cloud and Umbraco.Deploy.Forms have been patch updated to v9.0.1. When I locally revert these to v9.0.0 much of it this started working again.

I wonder if these have any specific changes or could change/overwrite some JSON configuration?

[p-m-j]

@bjarnef I'm not 100% certain this closes #11647 but #11448 was definitely a problem in core with ParameterSwapControllerActionSelectorAttribute.cs.

From the gifs it certainly looks very similar to #11448

Edit: changing package versions is interesting as it requires app to restart, the issue with ParameterSwap was that the request body was cached across requests and restarting could make pages that didn't work before start to work.

[bergmania]

Looks good and works as expected 💪

[bjarnef]

Will this be fixed in a patch of v9.0.x as well? Or do we need to upgrade to v9.1.x? Currently it is not possible to upgrade to v9.1 via Umbraco Cloud portal, so I guess we need to do this manually.

What I think it as bit strange is that for the specific project I am working on I didn't had issues with MNTP in v8 or after migrated to v9.0.0, but after the project was patch updated. I couldn't see anything specific regarding this in v9.0.1 .. but besides that Umbraco.Deploy.Cloud and Umbraco.Deploy.Forms have been patch updated to v9.0.1
Unfortunately these repositories are not public, so I can't check if there might be changes which could affect the JSON serialization / deserialization?

Locally I get some JSON exceptions as mentioned in #11647
The strange thing if I revert Umbraco.Deploy.Cloud and Umbraco.Deploy.Forms to v9.0.0 it seems MNTP stuff starts working again.
@bergmania @p-m-j could you check if there was made any changes made in Umbraco.Deploy.Cloud or Umbraco.Deploy.Forms which could collide with the MNTP or the JSON serialization / deserialization? It may only be and issue on Cloud as I guess these packages are not used on non-Cloud projects.

[p-m-j]

@bjarnef Existing v8 sites wouldn't have issues with GetUrlsByUdi's as it has never and will never exist there, instead a call for GetUrlById was made for each entry in the MNTP.

This method would have been introduced in 8.18 but now is replaced by GetUrlsByIds along with a bugfix for ParameterSwap attribute.

These fixes are going into 9.1.2

I don't see how Cloud/Forms would impact modelbinding in CMS, imagine the restart is just clearing the cached request body in ParameterSwap attrbiute.

[bjarnef]

@p-m-j I originally started the project on v8.15.1 and then migrated to v9.0.0, but I don't recall these issues with MNTP after migrating.

[p-m-j]

Which issue there are two here.

The introduction of GetUrlsByUdi to improve perf for MNTP was 9.1 #11207 (well this is the 8.18 version, it got merged into v9 some other commit) in and fixed in 9.1.2
The ParamaterSwap caching issue has been around since before 9.0.0-rc001 and is also fixed in 9.1.2

[bjarnef]

@p-m-j okay, I will upgrade of the the project when v9.1.2 is out. However many existing projects on v9.0.0 or v.9.0.1 won't get this fixes automatically and requires manual action to get these bug fixes.

Not sure why I didn't noticed any of these JSON exceptions when working on the project for a while on v9.0.0 though.

[p-m-j]

@bjarnef WRT

Not sure why I didn't noticed any of these JSON exceptions when working on the project for a while on v9.0.0 though.

It's not obvious unless you have both pickers for both int and udi mntp on same page (or just in the same site and have visited both pages)

Steps to reproduce in 9.0.0

1. Create a doc type with an MNTP, create a content with it and populate the MNTP with a single entry.
   

2. Edit and resend the request swapping the udi for the int id.
   

3. Submit - note 500 response
   

What's happening here is that the action constraint says yup I have a valid udi array here (from last request) you can use that action method, then modelbinding takes place and it says nope that's an int[] not a udi[]

[p-m-j]

If you have multiple udi pickers, the contraint says yup can use that action method as request has array of udi's and modelbinding agrees.

second request also works but the model binder and the action constraint were looking at 100% different strings so it's luck that it works.

[bjarnef]

In this specific project it shouldn't be an issue with int id I guess, since it only store udi. The MNTP datatype instances are mainly multi picker and one or two pickers on a document type or a used on a single property in en block element.