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.54k stars 2.71k forks source link

Media thumbnails not shown after 7.14.0 -> 7.15.0 upgrade #5847

Closed simonhawes closed 3 years ago

simonhawes commented 5 years ago

It appears in v7.15.0 the ImageProcessor now uses cmsMedia.mediaPath to display thumbnails. In many cases this field is NULL when images have been imported into Umbraco in a previous version.

This is characterized by angular reporting "Cannot resolve the file url from the mediaEntity, it does not contain the required metaData" in the developer tools console window.

In order to make the thumbnails appear correctly I had to write some code to update the mediaPath column from the cmsPropertyData table. The data exists in either the dataNvarchar or within a json string in the dataNtext.

This should be taken care of as part of the upgrade process ?


_This item has been added to our backlog AB#7181_

nul800sebastiaan commented 5 years ago

Can you show what you had to update? And where do you see errors/problems? Not sure why cmsMedia has nulls unless it's a media item without a filled in umbracoFile property.

Shazwazza commented 5 years ago

I think the key to understanding this is this:

In many cases this field is NULL when images have been imported into Umbraco in a previous version.

Which sounds like potentially very old versions of umbraco before there was a cmsMedia table that tracked media paths?

simonhawes commented 5 years ago

Here is the SQL I used (I had to code it up in C# in order to process the json variants, but this is effectively what I did)

UPDATE [cmsMedia] SET mediaPath = cmsPropertyData.dataNvarchar
FROM
(
              select contentNodeId, dataNvarchar from cmsPropertyData
)cmsPropertyData

WHERE mediaPath IS NULL AND cmsMedia.nodeId = cmsPropertyData.contentNodeId

The site was new at 7.14.0 so it is not that it was an old version. If you edit the thumbnail and save the node, the mediaPath is then added to the database correctly, but with thousands of images obviously this is not practical.

The document type contains a gallery property of type Umbraco.MediaPicker2 with “Pick Multiple Items” selected.

Our production site, which has not yet been upgraded (still 7.14.0), shows the thumbnails correctly without all the extra data in the mediaPath column. When I upgraded the staging version (which points to a very recent copy of the prod database, less than two days old) the thumbnails disappeared. After a bit of SQL’ing I took a guess at what was missing, updated a few manually which fixed the issue then wrote the code to update all rows in cmsMedia where mediaPath was NULL.

I can see the angular error “Cannot resolve the file url from the mediaEntity, it does not contain the required metaData” comes from here

https://pastebin.com/pVrdnci5

but have not yet delved any deeper into the umbraco source code. I got our content guy to create a new gallery, they just use the standard media picker, no import tools or anything like that.

simonhawes commented 5 years ago

Just looking into this further, I think I can see why the mediaPath was NULL. My predecessor on this project was bsowa who asked a question relating to issue

https://github.com/umbraco/Umbraco-CMS/issues/2943

@nul800sebastiaan replied with a code snippet to replace the SetMediaNameToFilename

This code snippet is being used by our solution but is missing setting the media.Path property at the end. I think we need this

                fileName = path.Substring(path.LastIndexOf("/", StringComparison.Ordinal) + 1);
                media.Name = fileName;
                **media.Path = path;**

I have not tested this, just reading the code, but I believe this is where our NULL mediaPath DB entries have originated from.

This still does not explain why the thumbnails were appearing in 7.14.0 and not in 7.15.0 though

nul800sebastiaan commented 5 years ago

To explain why this might have worked in 7.14: in 7.15 we updated a lot of the underlying code for retrieving media items because in 7.14 it was giving back ALL of the properties of each media item in the media picker. That was way too much info and with the introduction of the feature from PR https://github.com/umbraco/Umbraco-CMS/pull/2441 we did not want to expose the full set of data to all editors.

So it looks like we might need a migration that populates the mediaPath during an upgrade. I'm a bit concerned about @JamesSherborne's site though, as he mentions in #6106 he has 17000 items that need an update, not sure how well that will perform.

stevemegson commented 5 years ago

@JamesSherborne, could you post your FileSystemProviders.config, redacting the connectionString value? It might be that a fresh install with the default config works OK, but you've inherited config from an older version of the package.

Also, can you tell whether older media items have their mediaPath populated and newer ones are broken, or vice versa? (I'm going to optimistically assume that it's one of those, and not a random mix of populated and null).

JamesSherborne commented 5 years ago

@stevemegson attached is the FileSystemProviders.config

With regards to the mediaPath, I have just added a new image to test. Steps:

The cmsMedia table shows that this item does not have a mediaPath. It is not selectable in the Media Picker.

Conversely, if I follow the same steps as above but using the "original" Image Media Type the node does get a mediaPath and is selectable. The "original" Image Media Type again uses an Image Crop for the file upload. I say "original" as it may well have been amended at some point but is the "Image" Media Type which comes OOTB.

Differences:

FileSystemProviders.log

stevemegson commented 5 years ago

It'll be the property aliases causing the problem. mediaPath is being populated assuming that it comes from the umbracoFile property and starts with /media/. This causes further problems if your media picker has "Disable folder select" enabled. The media picker's test for isFolder is effectively "has no mediaPath", making any affected media items behave as folders.

As noted in comments in the code, v8 should really allow each property editor to determine whether it represents the mediaPath and what that path should be. A quicker fix for 7.15, and perhaps a temporary fix for v8, would be to recreate the logic from 7.14. That is, find the first Upload or Image Cropper property and use that path.

That would deal with new media items, leaving the problem of existing NULLs in the database. There was a migration in 7.8.0 to populate the mediaPath column for all existing media, but it made the same assumptions about only checking umbracoFile. It could presumably be modified to check all Upload and ImageCropper properties, though performance would certainly be an issue with thousands of items.

JamesSherborne commented 5 years ago

@stevemegson if I were to rename the aliases in my project to umbracoFile, is there a way for me get Umbraco to create the mediaPath automatically?

stevemegson commented 5 years ago

@JamesSherborne in principle you could make 7.15 think that your database needs upgrading from pre-7.8.0, then the "upgrade" would recreate the cmsMedia table. Something like:

JamesSherborne commented 5 years ago

@stevemegson tahnks, looks like your proposed steps do the trick. Umbraco seems to have no problem parsing all of the images but I now only have 6000 rows in the cmsMedia table rather than the 23000 I started with. Is this because it fails to process all or does the migration perform some housekeeping?

stevemegson commented 5 years ago

I guess the migration won't have recreated rows for any old versions of your custom items, since those versions didn't have an umbracoFile property. As long as it's created rows for all the current versions, that shouldn't matter.

stevemegson commented 5 years ago

PR #6195 implements the 7.14 behaviour for 7.15, with a migration to fix up the cmsMedia table for existing media.

PR #6221 does the same for 8.1, and also provides a way for other property editors to supply the media path.

andemt commented 5 years ago

@stevemegson Hi, I have the same problem (i think) with 7.15.3. In Umbraco admin ui, my image-property (Umbraco.MediaPicker2) doesnt have an thumbnail.

No warning in developer console, but I can see that image.thumbnail = "" with ng-inspect/AngularJs

In my old version 7.14.0: image.thumbnail = "/umbraco/backoffice/UmbracoApi/Images/GetBigThumbnail?originalImagePath=xxx"

When will this be fixed? I need to tell my client :)

RachBreeze commented 4 years ago

I have the same problem as @stevemegson on 7.15.3 but only on using an Azure WebApp with blobstorage. I have seen something similar for missing images in the back office and have checked the web.config in the media folder. Sadly the fix I used in 7.15.2 for when images weren't displaying at all, is in this folder, so the issue is just for thumbnails too:

<?xml version="1.0" encoding="UTF-8"?>
<configuration>
  <system.webServer>
    <handlers>
      <clear/>
      <add name="StaticFileHandler" path="*" verb="*" preCondition="integratedMode" type="System.Web.StaticFileHandler"/>
      <add name="StaticFile" path="*" verb="*" modules="StaticFileModule,DefaultDocumentModule,DirectoryListingModule" resourceType="Either"
        requireAccess="Read"/>
    </handlers>
  </system.webServer>
</configuration>
banarualexandru commented 4 years ago

Hi @RachBreeze I've migrated to 7.15.3 and have the same problem. Do you know any workarounds? I've tried @stevemegson solution by removing the cmsMedia table but didn't work for me. Thanks

RachBreeze commented 4 years ago

Hi @banarualexandru
We haven't been able to find a work around no. It's only on sites with blob storage, for sites which run on a VM/ local environment it's fine

RachBreeze commented 4 years ago

Further investigation shows this is only an issue for us when we have a different from expected container name in the blob storage connection ( we need to do this as \media maps to media held in umbraco v6 and \media-a maps to media held in Umbraco v7) So our v7 settings are: security.config image and our FileSystemProviders.Config is: image

I've stepped through the code and can see that when FileExists is called in Our.Umbraco.FileSystemProviders.Azure it's passing in the wrong url.
image

RachBreeze commented 4 years ago

PS there's also umbracoMediaPathweb.config property that can be set eg: <add key="umbracoMediaPath" value="~/media-a"/> This too didn't work

bowserm commented 4 years ago

I'm experiencing this issue on an Umbraco 7.15.3 site with Courier 3.1.7.0.

Everything is fine on the authoring site. I can create a new media node, see the preview thumbnails, and see the corresponding entry in the cmsMedia table in the database. However, when I attempt to right click deploy the media from authoring to production, the cmsMedia entry is not created on the production site which results in broken preview thumbnails for the backoffice.

The site has always used the default umbracoFile upload alias. It is using an ImageCropper property editor instead of an Upload. It isn't configured to store media in Azure blob storage. I just did a file compare of the /config/imageprocessor/* files. The only file with any differences is the /config/imageprocessor/processing.config and the differences are the addition of a bunch of enabled=true attributes and whitespace.

It seems like a similar problem. Should we discuss it here or should I create a new issue for it possibly as a Courier issue? Thanks!