Support for filefield

Patch looks great.

Does this create an uuid for all files or just the one handled by fielfield module?

I think its better to create for all files, so it would work for update module and others as well, if it's feasible.

If it creates for other files than those handled by filefield, the settings wording should be changed to cover that.

Since drupal 6 core doesn't provided any file api hooks, filefield invokes hooks similar to drupal 7's hook_file_*. This means that none of the core file handling is hook-able.

However, since files end up in the files table, this patch does enable storage of uuid's for all files managed by drupal.

For files uploaded using the core upload module, there may be something we can do in nodeapi to handle those.

Reviewed and works as advertised.

RTBC

Here's the patch rerolled against the latest code from CVS.

These patches look good, but it makes me wonder, for the long-term, do we really want to be creating a table for every type of object in drupal we want to add uuids to?

If we had one table: uuid, with the following structure:

objectid, type, uuid

where objectid could be a nid, a uid, a fid, a vid, whatever. Would not be unique, since it will be for any number of object types. Could be a taxonomy id, etc.

The type column would be an enum field (taxonomy, user, node, revision, file), and of course uuid would be the uuid for the object.

One advantage: by setting the uuid column to unique, we would force the application to only insert unique values across ALL types of content.

Second advantage: Easier to update and maintain the schema for the uuid module rather than having to create tables for every type of content we want uuids for.

Disadvantage: slower uuid lookups.

@klktrk: the slower uuid lookups could be improved, and mostly negated, with an index on (type, uuid).

Thanks DamienMcKenna. You make a good point.

Regarding Pedro Lozano's filefield patch, this line:

+  $result = db_query("SELECT fid, status FROM {files} WHERE status = 1 AND fid NOT IN (SELECT fid FROM {uuid_files})");

should probably be rewritten with a left join. The NOT IN (subquery) syntax does not scale well. Consider how that query will look once uuid_files begins to have more than a hundred rows.

I've made comments/patches about this issue, as it is widespread in the uuid module. You can see my related comments in issue #920052

Any idea if this fix will be incorporated into the next release? I'm eager to utilize uuid & uuid_features on a project but the filefield bug is a showstopper.

subscribe

I need this functionality for a site launching soon, so I'm trying out this patch... I re-rolled it with some fixes/tweaks:

* I'm using the latest dev release of UUID, which already has update_6004 and 6005, so I raised the update function # to 6006.

* modified the query with LEFT JOIN per apotek's suggestion

* checking uuid_automatic_for_files variable in uuid_sync() before creating file UUID's.

I'll test this now...

Ok, that seems to make sense having support for file fields.

I want to commit this patch, but this kind of constructor need to be changed from:

function uuid_file_insert(&$file) {
  if (!variable_get('uuid_automatic_for_filefield', FALSE)) {
    return;
  }

  _uuid_file_insert_uuid($file);
}

to:

function uuid_file_insert(&$file) {
  if (variable_get('uuid_automatic_for_filefield', FALSE)) {
    _uuid_file_insert_uuid($file);
  }
}

Please change this on all functions that has a check and a return, like this one above.

I updated the latest patch with comment #16 from recidive. If anything has to be changed let me know.

Seems to be working well for me.

is this in the latest dev? I doesn't apply.

; Information added by drupal.org packaging script on 2011-06-21
version = "6.x-1.x-dev"
core = "6.x"
project = "uuid"
datestamp = "1308617064"

patch -p0 < uu*h
patching file b/uuid.admin.inc
Hunk #1 FAILED at 79.
Hunk #2 FAILED at 153.
2 out of 2 hunks FAILED -- saving rejects to file b/uuid.admin.inc.rej
patching file b/uuid.install
Hunk #1 FAILED at 32.
Hunk #2 FAILED at 90.
Hunk #3 FAILED at 182.
3 out of 3 hunks FAILED -- saving rejects to file b/uuid.install.rej
patching file b/uuid.module
Hunk #1 FAILED at 306.
1 out of 1 hunk FAILED -- saving rejects to file b/uuid.module.rej

Edit: I think applying patches has changed.
I used
git apply path-name and it was just fine.

I apologize that I did not know the difference before I posted this.

#18: uuid-files-808690-18.patch queued for re-testing.

in uuid_file.features.inc, function uuid_file_features_rebuild() -> what is the point for the DELETE db query? it seems to make no sense. In the process of the feature 'revert' you wipe out the uuid_files table which has the fid/uuid mapping and then in a later function (modules/filefield.inc) you attempt to load the file info based on that table (this happens in filefield_uuid_node_features_rebuild_alter()).

Seems like a bug to me which then I don't know how some guys here reported this is working well (it doesn't just 'seem', it's actually not working for me unless I comment that DELETE query).

#1: uuid-files-808690.patch queued for re-testing.

i don't think the DELETE query should be causing you trouble - note that at the end of the uuild_file_features_rebuild function uuid_set_uuid is called, which adds an entry back into the uuid_files table. and uuid_set_uuid is smart enough to try UPDATE first and then INSERT if UPDATE doesn't work, so it should literally make no difference whether you comment out that DELETE or not. if commenting it out is changing functionality for you, then either i'm fundamentally failing to grasp something here, or one of those queries is written wrong.

actually, pardon me - the DELETE query in question should indeed be removed, but for a different reason - it can throw a warning if there's not actually a record to delete, which is true the very first time you install the feature (and as I noted, the query is otherwise irrelevant anyway, because if the record isn't deleted it'll just get overwritten later in the function).

Hi, I've tried applying this patch and I get errors. Can any one help as I think without this files will not work. I have tested features and it imports the nodes fine but I dont get any files, and I think its due to this patch not being applied. Can someone give a little more detail as I think this feature uuid is awesome! thanks

What's the status of this? We're coming up on two years since this was first posted. I was able to apply the patch manually quite easily and am now testing it.

On testing this, it appears from the import of the feature to a new server that it is expecting to have a folder called uuid_files within the exported feature module containing the files renamed with the UUID. I am not seeing this in the export, nor do I see anything in the code that would create this. Does this also depend on an update to features, or is there something still missing from this patch?

Is the work here only going against D6? (Is this supported already in the D7 branch, or is D7 core enough by itself for the UUID/Filesfield functionality?)

UUID for D7 supports file entities out of the box.

Installation Instructions

Install this patch before enabling the module. Alternately, uninstall the module, apply the patch, then install it again until you see the 'files_uuid' table present.

18: uuid-files-808690-18.patch queued for re-testing.

15: uuid-files-808690.patch queued for re-testing.

14: uuid-files-808690.patch queued for re-testing.

Re: #31 - the patch definitely needs an update hook then - shouldn't require uninstall and reinstall as that destroys existing data.

Drupal 6 core is no longer supported. We are no longer supporting 6.x-1.x versions of this module. I am closing this issue as won't fix.