Comments

JacobSingh’s picture

This is actually kinda hairy.. :(

I'm looking at the code from http://drupal.org/files/issues/file_usage_19.patch in file.field.inc.

The media field's implementation is not nearly as robust w.r.t. revisions on entities. I'm wondering if the best approach is to switch the media field to being a widget of filefield. This presents its own issues of course.

I wonder if http://drupal.org/project/filefield_sources is a good solution. This of course is a radical departure of the media module architecture, but is a nice similar module. I guess my reservation here is that one of media's goals is to have the same interface everywhere, for WYSIWYG, for fields and for custom implementations...Anyway, I digress...

JacobSingh’s picture

Title: Implement the new file_usage API » Implement the new file_usage API (delete no longer works)
JacobSingh’s picture

filefield sources doesn't have a D7 version though, and porting it would be a kinda crazy endeavor.

effulgentsia’s picture

Title: Implement the new file_usage API (delete no longer works) » Implement the new file_usage API

The "delete no longer works" part of this issue title is a separate issue: #911830: Can't delete any media item, because all are supposedly in use.

But for this issue, we do need to implement media_field_(insert|update|delete)() to call file_usage_(add|delete)(), much like file_field_(insert|update|delete)() does.

The media field's implementation is not nearly as robust w.r.t. revisions on entities.

I don't think revisions has anything to do with this. Files are not revisionable. From the perspective of file usage, media entities are just files, so media entities not being revisionable is not an issue. The media field should work when attached to a revisionable entity (since attaching media fields to nodes is one of the main use-cases), so the new media_field_(insert|update|delete)() functions should be written to work when the field is attached to a revisionable entity, but hey, the file_field_*() functions already figured this out, so we can copy them. It does suck duplicating code though. It's interesting that the image_field_(insert|update|delete)() functions just call the file_field_*() ones, so bonus points if media field can be made to do the same: that may require submitting a core patch to file.module to not hard-code assumptions it shouldn't make, but such a patch could be a good thing for other contrib field modules that are kind of like file field but aren't file field.

I'm wondering if the best approach is to switch the media field to being a widget of filefield. This presents its own issues of course.

As per above, even image field is its own field type and not a widget of file field. So, I don't think it's necessary to shy away from media field continuing to be its own field. It should be possible for contrib modules to define field types that work with files without duplicating 100 lines of file.module code. If that's not possible, we need to be submitting patches to improve file.module while we still can. I can see the appeal of wanting to merge file field and media field into one though, given that we simply want all files to be entities. But I don't think we can do such a thing successfully at this time, because we still need the freedom to experiment and try out code in media.module that we won't be able to get into file.module until D8.

filefield sources doesn't have a D7 version though, and porting it would be a kinda crazy endeavor.

I'd love it if we could organize an opportunity to chat with quicksketch and other developers interested in filefield_sources and related modules. Given the state of media for D6, those modules were necessary, but ideally, D7 media could solve everything those modules did, so rather than spending time porting those modules, maybe we can attract their developers to help out on media. Given D7's entity system, I'm just not sure why there's a reason to have a non-entity-based file reuse architecture, which is what D6 filefield_sources is. Why wouldn't D7 filefield_sources want to use entities? And do we really need two totally different entity-based file reuse architectures, or can we combine efforts here?

Anyway, I digress. The focus of this issue is to implement media_field_(insert|update|delete)() to call file_usage_(add|delete)().

effulgentsia’s picture

I'd love it if we could organize an opportunity to chat with quicksketch and other developers interested in filefield_sources and related modules.

Well, here's an attempt towards that: #864800-3: FileField Sources: D7 version.

JacobSingh’s picture

Status: Active » Needs review
StatusFileSize
new4.09 KB

This doesn't do a usage check on delete, but implements the file_usage management stuff.

effulgentsia’s picture

Status: Needs review » Fixed

90 lines of nearly identical code to what's in file.field.inc. So sad. But looks good, and the duplication is needed until we can make the core functions more flexible. Committed #6 with the addition of this comment:

/**
 * @todo The following hook_field_(insert|update|delete|delete_revision)
 *   implementations are nearly identical to the File module implementations of
 *   the same field hooks. The only differences are:
 *   - We pass 'media' rather than 'file' as the module argument to the
 *     file_usage_(add|delete)() functions.
 *   - We do not delete the file / media entity when its usage count goes to 0.
 *   We should submit a core patch to File module to make it flexible with
 *   respect to the above, so that we can reuse its implementation rather than
 *   duplicating it.
 */

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.