Provide an upgrade path for media types to file_entity types

The patch applies cleanly. Works great for me, combined with the file_entity patch at http://drupal.org/node/1292382#comment-5771928

Marking this RTBC based on irc conversation.

I get the following error when applying patch in #6 on a current 2.x dev and running the update.php. I'm testing this patch in combination with patch in #62 0f the #1292382: Make it possible to create any number of custom File Types issue:

media module
Update #7203

    Failed: PDOException: SQLSTATE[23000]: Integrity constraint violation: 1048 Column 'type' cannot be null: INSERT INTO {file_type} (type, label, description) VALUES (:db_insert_placeholder_0, :db_insert_placeholder_1, :db_insert_placeholder_2); Array ( [:db_insert_placeholder_0] => [:db_insert_placeholder_1] => Audio [:db_insert_placeholder_2] => ) in file_type_save() (line 535 of [path]/sites/all/modules/file_entity/file_entity.file_api.inc).

It did 2 updates to the File Entity module, but this one for Media simply wont go through. I have tried re-running update.php and then its only the above update left, but it still fails.

Oki, done some more testing. This time on a new installation with this and #1292382: Make it possible to create any number of custom File Types applied before installing the site. Installation went fine without any errors, but I get another PDOException when trying to edit a field after setting the widget type to Media file selector.

PDOException: SQLSTATE[42S02]: Base table or view not found: 1146 Table 'media-2-dev.media_type' doesn't exist: SELECT mt.* FROM {media_type} mt ORDER BY weight ASC; Array ( ) in media_type_get_types() (line 95 of [path]/sites/all/modules/media/includes/media.types.inc).

Steps to reproduce:

1. Install 7.12 Standard profile
2. Enable Media/File Entity modules with patches
3. Change the Widget type for Article/Image to Media file selector
4. Try Operation/Edit for Image
5. Should get similar error as above

It is possible to switch back to the Image widget and then access the Edit page again. So clearly something is going wrong in this patch.

This is already fairly old. Has the development in the meantime made this superfluous?

#6: remove-media-types-crud-1496624-6.patch queued for re-testing.

i think we can just delete media.types.inc
this needs quite some work yet, but we should definitely make it happen before next unstable release

Ok this is pretty much ready :) removes quite some cruft
Just need some help here

+      // @TODO: Perform some regex magic on the mimetypes.
+//      foreach ($mimetypes as $mimetype) {
+//
+//      }

feel free, regex is not my thing

rerolled

per #12

After talking with dave in IRC, there is no need to remove update functions and force users to update to latest 1.x version before going to 2.x

here is the patch.
still needs #12 to be resolved

ok and this is the complete patch..turned out to be simple.

please rtbc, without this all installations prior to latest devs are broken

Other media type is causing problems with this approach.
copy-paste from irc:

in file_entity there is no "weight" field like media types had..and because other type was matching all the elements, now when you upload a new file, whatever mimetype that is, it gets the "other" type instead of image

Another approach, see #1808902: Broken / incorrect file types after recent upgrade, credits @mrfelton

.....and complete patch:

Marked #1813300: File Type undefined as a duplicate.

Ok i removed media type crud code, but we still need the upgrade path.
Updated patches of #16 and #18 with the two different approaches..much more easier to review now:

oops, wrong component

I'd be willing to test this but don't really understand the two patches..are they alternate/different approaches to doing the same thing? Do I only apply one of them?

Yes.

First patch migrates old media types to file entity types, but "Other" media type screws everything because file_entity types do not have weight to make it apply last.
Second patch just drops the media_type table without migrating media_types and just updates existing files (their type property) to the new file_entity default types.

If we go with the first way we would definitely need a separate rule for "Other" media type. ie, dont migrate it at all.
If we go with the second we need to check if there is any other type besides the default media types that shipped with media (which i really doubt. does any module provide anything like that? i think no)

So in my opinion second way is the way to go

I saw this node in the commit history and didn't realize you had only committed the patch in #18 to .dev. I'll apply the second variant and test it presently.

Updated patches

Yeah second was the old one

I used the second variant together with latest dev version of module and it worked fine for me (fixed this problem http://drupal.org/node/1792738#comment-6618934). Got the result I was expecting.
Thanks for the good work!

Applied the second patch and it updated all my types except for some Vimeo videos...they're still 'undefined'

I also just added a new youtube video and it's type was "undefined" as well....so new content is also having the same problem? (I realize this isn't part of this 'update' issue, but it sheds light on maybe why the patch didnt work for me)

It looks like this was my issue with the patch, so ignore my objection but please re-review with functional offsite videos: #1812976: Register video/youtube mimetype and provide an upgrade path

switching back to 'needs review' since after the patch from the issue I listed in #28, this update action worked across the board.

I had to modify the update number from 7207 to 7208 since there is now a 7207 in the git repo. Rerolled the patch and attached.

The good news: when I tested this patch on a site that was on unstable6, with over 5000 files, including images, audio, video, and documents, the update was successful!

Should probably have one more reviewer, but I think this is RTBC.

I tested this patch and the upgrade worked well

New patch that merges old mimetypes to new mimetypes as well

didnt test it yet btw

Based on my review, I think that regardless of the mimetype mapping solution, an error condition exists for a failed mapping. The update should probably fail (because this type of update is not re-runable due to the table delete) or at the very least return an error in that case.

sorry, did not mean to change the status

Ok..i searched a bit more...and docx not appearing as correct filetypes is not in the scope of this patch...There is not even a file type for application in {media_type} table which gets dropped..
Can you confirm that before upgrade there is no application type entry, so i commit this?

You're correct, there's no file type for application in {media_type}. However, other files (mainly pdf) get converted from application to document, and these should too.

That said, the issue may actually be elsewhere (in file entity, perhaps?) and have nothing to do with this patch. The mime type that failed was for a MS Word file with extension .docx. The mime type itself was:
application/vnd.openxmlformats-officedocument.wordprocessingml.document

I see that file_type_get_enabled_types() does not return the "application/vnd.openxmlformats*" types. For "document", it only returns:

            [mimetypes] => Array
                (
                    [0] => text/plain
                    [1] => application/msword
                    [2] => application/vnd.ms-excel
                    [3] => application/pdf
                    [4] => application/vnd.ms-powerpoint
                    [5] => application/vnd.oasis.opendocument.text
                    [6] => application/vnd.oasis.opendocument.spreadsheet
                    [7] => application/vnd.oasis.opendocument.presentation
                )

If you look at includes/file_mime_types.inc, there are A LOT more application/xxx mimetypes, none of which are returned by the above function. I think that is the root of this problem. Where would be the appropriate place to file an issue? Is this File Entity?

So anyway, I agree it has nothing to do with this patch. Why I marked it as "needs work": as kpolymorphic wrote in #34, there is no error message when there is a failure to map a file, and there probably should be. Make the failure visible, and that should allow this fix to be RTBC.

Thanks for your excellent work!

sorry i missed this..like a drupal_set_message you mean?
i am planning to add an update function to file_entity for documents types before unstable7, that include all application/* mimetypes so yeah it should be fixed there

Since it is an implementation of hook_update_N, if you return a string, that string will be displayed to the user on the update result page. It will also get displayed if the user uses drush to update the database.

So you could build an array of error message strings, implode it with PHP_EOL, and return it.

Great news that the missing mime types will be fixed in file_entity.

Cheers!

Ok, great thanks for explaining:)

Gave it a more thought and decided that this application => document is a media thing afterall..so here is a new patch..includes two update functions..once migrates types and fixes document the other updates fuiles and returns a list of the failing in the end:)

I think this is so much better now...thanks for the help all!

This patch turns failed file types to FILE_TYPE_NONE, so any module (eg media_youtube) can update those files without scanning the whole table again..
just using WHERE type = 'undefined' in its query.

Will try this patch now on a site with 11k files..If it works then i will try on it the #1349058: Convert 'Media' fields to 'File' fields that use the 'Media file selector' widget..if both goes well i will commit them and then release unstable7..
i think its about time

commited with some fixes on comments and return text.
see here http://drupalcode.org/project/media.git/commit/1233117

Thanks for the testing all!

Uh oh...

I pulled the latest file_entity and media from 7.x-2.x git. Updating from unstable6 failed for me. Here's the output from drush updb:

$ drush updb
The following updates are pending:

file_entity module : 
  7201 -   Add the {file_type}, {file_type_mimetypes} and {file_type_streams} tables. 
  7202 -   Drupal 7.8 disallows empty string as the value for a bundle key, so update  empty {file_managed}.type records to 'undefined' instead. 
  7203 -   Update permission names. 

media module : 
  7203 -   Empty update function to trigger cache clear. 
  7204 -   Update old Media view modes to the new File Entity ones. 
  7205 -   Drop the unused {media_list_type} table. 
  7206 -   Move default file display configurations to the database. 
  7207 -   Empty update function to trigger cache clear  after changing access callbacks to file_entity_access. 
  7208 -   Drop the media_types table and migrate all files to the new file_entity types. 
  7209 -   Update {file_managed}.type with the new file types provided by file_entity. 
  7210 -   Delete deceprated media__type_icon_directory variable. 

Do you wish to run all pending updates? (y/n): y
Performed update: media_update_7203                                                                [ok]
Performed update: media_update_7204                                                                [ok]
Performed update: media_update_7205                                                                [ok]
Performed update: media_update_7206                                                                [ok]
Performed update: media_update_7207                                                                [ok]
SQLSTATE[42S02]: Base table or view not found: 1146 Table 'testsite.file_type' doesn't exist     [error]
Performed update: media_update_7208                                                                [ok]
Finished performing updates.                                                                       [ok]

It looks like the issue is with update 7201 in file_entity.install, so I'll write up a new issue in file_entity.

Actually, looking at it further, the problem is that file_entity_update_7201 had not been run yet. This is a new area for me, but I think you can use hook_update_dependencies (http://api.drupal.org/api/drupal/modules!system!system.api.php/function/...) to specify that update 7208 in media depends on update 7201 in file_entity.

I'd try to roll and test a patch, but don't have time right now due to family events today. If somebody else is not able to look into it and I can squeeze it in tomorrow, I'll see if I can roll another patch to media.install to check for that dependency. But in the meantime I thought I'd post what I figured out so far and see if somebody else has time to run with it.

Thanks paul!
I made media update 7208 dependent on file_entity 7201

Have a nice time to day:)
thats what i committed

http://drupalcode.org/project/media.git/blobdiff/c1804b9ba76210ecdc929b3...

@rootatwc, thanks for all your hard work.

Unfortunately, I found another issue. This time, videos are getting set to "undefined". Again, I'm upgrading from unstable6 to the latest from the git 7.x-2.x branch.

This site is using media_brightcove 7.x-3.4 and media_youtube 7.x-1.0-beta2, if it makes a difference.

Before update:

mysql> select filename, filemime, type from file_managed where type='video';
+---------------+------------------+-------+
| filename      | filemime         | type  |
+---------------+------------------+-------+
| 1147761052001 | video/brightcove | video |
| 1754174737001 | video/brightcove | video |
| 1828225758001 | video/brightcove | video |
| 1671308489001 | video/brightcove | video |
| 745816525001  | video/brightcove | video |
| 1147761047001 | video/brightcove | video |
| 1147761048001 | video/brightcove | video |
| 1147801310001 | video/brightcove | video |
| 1147801311001 | video/brightcove | video |
| 1870977046001 | video/brightcove | video |
| s05UjecdcGU   | video/youtube    | video |
| nxBqHg42dw8   | video/youtube    | video |
+---------------+------------------+-------+
12 rows in set (0.00 sec)

After update:

mysql> select filename, filemime, type from file_managed where type not in ('document','video','audio','image');
+---------------+------------------+-----------+
| filename      | filemime         | type      |
+---------------+------------------+-----------+
| 1147761052001 | video/brightcove | undefined |
| 1754174737001 | video/brightcove | undefined |
| 1828225758001 | video/brightcove | undefined |
| 1671308489001 | video/brightcove | undefined |
| 745816525001  | video/brightcove | undefined |
| 1147761047001 | video/brightcove | undefined |
| 1147761048001 | video/brightcove | undefined |
| 1147801310001 | video/brightcove | undefined |
| 1147801311001 | video/brightcove | undefined |
| 1870977046001 | video/brightcove | undefined |
| s05UjecdcGU   | video/youtube    | undefined |
| nxBqHg42dw8   | video/youtube    | undefined |
+---------------+------------------+-----------+
12 rows in set (0.00 sec)

Thanks,
Paul

Yeap.
For youtube videos try running the update with latest 2.x dev (see #1812976: Register video/youtube mimetype and provide an upgrade path)

media_brightcove needs to catchup as well...which project is is? http://drupal.org/project/media_brightcove only has 6.x versions

Those mimetypes are not provided by media module so these modules will have to provide upgrade paths

Thanks!

To answer your question, the brightcove project is actually just "brightcove" - http://drupal.org/project/brightcove. (I had mis-named the project in my earlier comment.)

FYI - I just downloaded the latest dev versions of Media and FIle Entity on a fresh install and tried to enable Media and have it enable File Entity as a dependency and it failed with

Module media doesn't meet the requirements to be enabled. [error]
Media 2.x requires File entity 2.x. Please download the correct version and make sure you have deleted the file_entity folder inside the media [error]
module directory. (Currently using File entity 2.x Wrong version)

I then enabled File Entity first, then was successfully able to enable Media.

You may have a problem with the install process.

thanks for the headsup
see here #1749252: Add check for File entity 2.x to installation requirements

When testing the upgrade path for the panopoly distro we ran into an issue with update 7208 which was introduced in this patch. The issue is the assumption of the media_type table existing. However, the media_type table isn't defined in media_schema, but rather is introduced in media_update_7002. Perhaps it was originally in media_schema but has since been removed. When the table doesn't exist it causes the update to break, and subsequent updates aren't ran.

The following patch just does a db_table_exists check on the media_type table before doing the heavy lifting in 7208.

I guess if this table does not exist then we could just return

Just commited a check in the start of the update. if table does not exist it returns

Yesterday i solved this problem updating media, media_youtube and media_vimeo to the latest devs.
Since then I have the problem described in this thread, specifically in my comment:

http://drupal.org/node/973202#comment-6761304

Yesterday i solved this problem updating media, media_youtube and media_vimeo to the latest devs.
Since then I have the problem described in this thread, specifically in my comment:

http://drupal.org/node/973202#comment-6761304

Any chance these could be related?