umbraco / Umbraco-CMS

Umbraco is a free and open source .NET content management system helping you deliver delightful digital experiences.
https://umbraco.com
MIT License
4.42k stars 2.67k forks source link

Media Picker V3 Models builder null exception #10416

Closed seanhakbb closed 3 years ago

seanhakbb commented 3 years ago

Which Umbraco version are you using?

8.14

Bug summary

I get exceptions from models builder just by calling the property get method (Model.Image).

When not having a image value its impossible to null check the property without try..catch or possibly Model.HasValue, Since Models builder just throws a null exception. I usually just check if media is set using Model.Image != null - this might not be the correct way since I cant find any one else with similair issues, and this one being so obvious.

Or is this more because I dont have any crops (global or local) setup?

Specifics

Models builder setup: [global::System.CodeDom.Compiler.GeneratedCodeAttribute("Umbraco.ModelsBuilder.Embedded", "8.14.0")] [ImplementPropertyType("image")] public virtual global::Umbraco.Core.Models.MediaWithCrops Image => this.Value<global::Umbraco.Core.Models.MediaWithCrops>("image")

Steps to reproduce

Steps to reproduce, upgrade a 8.13 site. Go to media picker datatype, and change it to Media Picker V3. Dont setup any crops. Leave pickers empty (in my case this was views in DocTypeGridEditor) Fix view code to reference Model.Image.MediaItem.Url(). When previewing grid, you will now get an error.

Expected result / actual result

No response

ronaldbarendse commented 3 years ago

Hi @seanhakbb! Looks like you're indeed missing a null-check: if you don't have a media picked, Model.Image will be null and thus Model.Image.MediaItem will throw a NullReferenceException.

@if (Model.Image != null)
{
   <a href="@Model.Image.MediaItem.Url()">@Model.Image.MediaItem.Name</a>
}

@* Or in a less verbose/more optimized way, using the null coalescing operator and pattern matching: *@
@if (Model.Image?.MediaItem is IPublishedContent image)
{
   <a href="@image.Url()">@image.Name</a>
}
seanhakbb commented 3 years ago

@ronaldbarendse Thanks for your reply! My problem was that I got exception from modelsbuilder just by doing that, Model.Image != null will cause the exception error. But if nobody else experience this it must be a unique scenario caused by my upgrade method.

ronaldbarendse commented 3 years ago

Ah, I see you're changing the editor of an existing data type from the 'old' Media picker to the new one: this isn't supported on 8.14. The existing data stored on the content/properties isn't in the correct format, which causes exceptions to be thrown in the property value converter (parsing the value into the MediaWithCrops model).

We've made changes for 8.15 to enable this upgrade path, check https://github.com/umbraco/Umbraco-CMS/pull/10517. I recommend leaving the editor unchanged for now and only migrate to the new media picker when this version is released.

seanhakbb commented 3 years ago

@ronaldbarendse Ahh! Thanks for that. Makes sense that the data isn't fully cleared in the database, just can't be displayed in the new v3.

Honestly after testing the v3 picker in a clean install, I see this less as a replacement and more as another usefull tool to the umbraco arsenal, a bit like blocks and nested content/grid.