Problem/Motivation
The media token upgrade mechanism scans all entries in the {file_usage}
table, but it's really only necessary to upgrade entries that belong to the 'media' module. This will in many cases drastically reduce the number of parsed entities during token upgrade.
I completely missed this little feat of the file_entity/media symbiosis.
Proposed resolution
Add a db condition for module == 'media'
in media_wysiwyg_upgrade_content_tokens_batch()
.
Comment | File | Size | Author |
---|---|---|---|
#13 | media-2962845-media-token-upgrade-used-by-media-only-4.x-4.patch | 810 bytes | kaare |
| |||
#3 | media-2962845-media-token-upgrade-used-by-media-only-3.patch | 707 bytes | kaare |
|
Comments
Comment #2
kaareComment #3
kaareThis one-liner will drastically reduce the affected entities in large installations.
Comment #4
joseph.olstadare documents and music files like .mp3 also type media? those can be embedded as well.
What about custom file types? I think those can be customized to be embedded, would have to double check.
Comment #5
kaareGood questions and reasonable concerns.
I'm quite sure this is valid for other file types as well. as long as the token
type == media
and it has afid
it's counted as a managed file by media module and'media'
is the module owner, as set by_media_wysiwyg_filter_add_file_usage_from_fields()
. Within it there are calls tofile_usage_delete()
andfile_usage_add()
, both with'media'
as module owner.But to be sure, I extended the media_dev distro with a custom file type and inserted files of this type both on nodes as stand alone file fields, and in other nodes as wysiwyg tokens. I also added mp3s both in standalone file fields and within wysiwyg areas: In the database the file usage table tracks these files in wysiwyg text areas as owners of
'media'
.I'm quite sure this patch is correct.
Comment #6
wellsI have tested this patch against my database, stemming from Issue 2950150. This patch resolves the issue for my environment and cuts the number of batched entities from 250k to 311. I spot checked many of the updated nodes and everything appears to be as expected.
Comment #7
kaareThank you @wells for your feedback and testing. This revelation came while investigating your issue and I'm glad you tested it in your large installation. I imagined this simple patch will omit the troublesome entities during upgrade in your case. The drop in parsed entities was enormous for your case, as suspected. I'm ready to call this rtbc.
Comment #8
joseph.olstadnice test results.
Just curious, did you have any other file entity content types created other than the default 'media' type (and configured for embedding)?
I imagine that those custom ones might not get upgraded in that case?
Comment #9
joseph.olstadlooking into this now
Comment #10
kaareYes I did. I created a file entity bundle (type) called 'patch' with text/x-diff as mime and .patch/.diff as additional extensions in file settings. It was added as an allowed type in WYSIWYG under
admin/config/media/browser
. In addition I created a new content type name'issue'
with a new file field named'field_patch'
and media browser as widget. I then tested creating a new node (of type 'issue') and uploaded a patch here. I created a new node (of type 'article') and in the 'body' using wysiwyg media browser I uploaded a new patch and inserted it as media token. All this done in the 7.x-2.x media branch. I then switched to the media 7.x-3.x branch with this patch applied and ran the upgrade. Everything worked as planned. The 'issue' nodes wasn't touched, the articles with the 'patch' file bundle as token was upgraded along all other tokens. I inspected thefile_usage
table and asserted that the different 'patch' files belonged to different modules, and that the one inserted in wysiwyg had 'media' as module owner.All good.
Comment #11
joseph.olstadya just tested, I created a new file type called xyz , configured it for images, enabled wysiwyg embedding, embedded it, the file_usage shows 'media' despite the file type/bundle being xyz.
so yes, this is a good patch thanks double confirmed!
I was curious too lol
Comment #12
joseph.olstadso we should also put this in the 4.x branch?
Comment #13
kaareYep. Forgot to add patch for that branch as well (code was moved to
.update.inc
Comment #15
joseph.olstadok, so RTBC patch #13 for 4.x
and RTBC patch #3 for 3.x
Munga takk!
Comment #16
joseph.olstadSo ya Kaare, I'm trying to stay out of your way in the 3x and 4x branches, great work!
Comment #19
kaare