Normally, deleting a node with a file field will delete the file itself, if that node is the only entity pointing to the file. This is often undesirable when migrating - the file might be from a mounted repository of files from the old system and pointed to by file_link, or it might have been imported by a MigrateDestinationFile migration. At /node/1061560 we documented using hook_file_references() to prevent this, but that hook was removed from Drupal 7 before 7.0 was released. So, we need another mechanism - my thought is to, in the presence of the appropriate option (for MigrateDestinationFile) or argument (for file fields), add a file_usage row so that the filefield reference will not decrement usages to 0 and thus the file itself won't be deleted on rollback.

Comments

moshe weitzman’s picture

moshe weitzman commented

That seems like the best solution. Hope there are no ill side effects.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Active » Needs work

Done for MigrateDestinationFile, still TBD for file fields...

btmash’s picture

btmash commented

I'm wondering if, for field fields, there could be another option that is a part of the json arguments that would / would not remove the file in the field upon rollback. In my scenario, I am copying the files into the files directory so they fully become a part of the Drupal installation so having the files get deleted would not occur. Though I don't know how far tied the migration and rollback are to each other to be able to gather such data. Or perhaps the inclusion of a rollback function in the classes that define any particulars to do / not do.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Fixed

@BTMash: Rollback doesn't have access to field-specific stuff - I think the original plan, to bump up file_usage on import, is the way to go.

And... I went that way, it's committed. This is the most important remaining item I wanted to get in for 2.2, although I may try to knock off a couple other small items before cutting a release candidate.

fearlsgroove’s picture

fearlsgroove commented
Status: Fixed » Needs work

Maybe I'm crazy but this doesn't seem to actually work. I'm testing with 2.x-dev, do a simple file MigrateDestinationFile, I pass preserve_files option. It does add the file_usage as expected, but a migrate-rollback still deletes all the files.

Am i missing something?

edit: Perhaps this only works if you ALSO do a follow up filefield import? If so I'd still leave this open pending a better solution. Especially given an option called "preserve_files", I'd expect to be able to import and immediately rollback a migration with MigrateDestinationFIle without it deleting my pre-staged files.

An alternative solution, tho obviously not a general best practice, would be to do a db_delete against the files record instead of a file_delete if we've got the $preserveFiles option set.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

Huh - now why are we doing this on rollback?

 file_delete($file, TRUE);  // 2nd parameter is $force

Actually, I know - before we went through all this, we had the opposite problem - rolling back failing to wipe things clean. So, that second parameter to file_delete needs to depend on the preserve_files option...

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

Oh, wait a minute - thought I could get away with just setting $force to !$this->preserveFiles, but looking at the actual file_delete code it's more complex than that. When rolling back with preserveFiles in effect, we do want to remove the DB evidence of the file, just not the file on disk, but file_delete is all-or-nothing in that regard. I think you've got the right idea, we've got to wipe the file_managed and file_usage tables ourselves. I don't have time at the moment to test this, I'll come back to it and do something like:

  • preserveFiles is FALSE - just call file_delete with $force=TRUE to wipe everything.
  • preserveFiles is TRUE - call our own version of file_delete that skips file_unmanaged_delete().

The question is, should we call the file_delete and entity_delete hooks in the latter case? Probably, but I want to take a look at what some typical hooks do just to be sure they don't make assumptions that the file must disappear...

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Fixed

OK, committed - yes, I think calling the hooks is the right thing to do.

geerlingguy’s picture

geerlingguy commented

I just encountered this on my site, when importing and rolling back nodes with files that I've imported previously. I'd rather not have the files be deleted when I rollback - how can I tell Migrate to do this? It seems a little ambiguous as to how to set that to happen in my migration class.

Also, so that we can help generations of future migration-minded people, it would probably be helpful to update the doc page Preventing file deletion on rollback...

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented
Status: Fixed » Needs work

Hmm. I think this only works if the MigrateDestinationFile is in play. That's not the case with a regular entity delete, true?

In my case, I have a Product entity with a Commerce File field. I don't think that MigrateDestinationFile is involved anywhere...

In this case, file_field_delete() does the dirty work.

My next experiment will be to experiment with a bogus file_usage_add() after the $file creation to hopefully prevent deletion from every happening.

      file_usage_add($file, 'AmazonS3', 'preexisting', $file->fid);
rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

OK, so I discovered that the approach in #10 had already been implemented in the earlier commit.

However, I notice two things about this approach in MigrateFileFieldHandler::prepare():

          if (!empty($arguments['preserve_files'])) {
            file_usage_add((object)$file_array, 'migrate', 'file', $file_array['fid']);
          }

1. Every time you do a rollback and import, it increments the usage counter. That's not right.
2. If I'm not mistaken, this file will never be able to be deleted by any technique, because migrate has its usage counter in there. Is that ever cleaned up?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

@geerlingguy, I updated http://drupal.org/node/1061560 to what I think it should say.

@mikeryan, would appreciate it if you'd check that and revert my edit if it's wrong.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

Yes, it should recognize whether this is the first time we've migrated the file, and only increment it then, I'll fix that before the final 2.2.

Yes, if you've said to preserve_files, they're preserved forever. In what circumstances, having indicated you want to preserve the files, would you then want to remove them? Recall the original context, where the files are being externally copied into place, or are linked from a read-only mount - in those cases, you never want to try to remove them. In what other circumstances are you using preserve_files?

rfay’s picture

rfay
he,him
Primary language English
Location Palisade, CO, USA
commented

@mikeryan: You might want to delete the files someday manually after the migration! Using the regular UI. You wouldn't be able to, unless migrate removes its claim on the file.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented

I'm going to postpone dealing with usage count issues until after 2.2 - refactoring the file support will be the main goal for 2.3.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Needs work » Postponed
Issue tags: +Migrate 2.4

Will be dealt with for Migrate 2.4, under #1240928: META: Refactoring of file destination/field handlers.

mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Status: Postponed » Active
mikeryan’s picture

mikeryan
he/him
Primary language English
Location Pittsfield, MA, USA
commented
Assigned: Unassigned » mikeryan
Status: Active » Fixed

OK, I've committed a couple of improvements here with file fields - now it explicitly sets the count to 1 every time, so you don't get extra increments on updates, and the uninstall hook now removes all migrate-owned rows from file_usage.

I think this is about all we can do in Migrate itself - if you need a different preservation behavior for a particular application, I believe there's enough flexibility (through handlers and/or overriding the file class) to achieve that.

Status: Fixed » Closed (fixed)

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

cameron tod’s picture

cameron tod commented

I think I came up against this again today. If I Import, then rollback, then import, then rollback, then import again, I get a duplicate key error when imporiting a file into the field.

On the second import, this is what my file usage table has in it:

fid	module	type	id	count
250	migrate	file	250	1
250	file	node	1141	1

On the second rollback, the file/node usage is removed, but the migrate/file remains, so the file record is never deleted in the file_managed table, and on the next import there's a duplicate key error.

I'm not sure what the best approach is to addressing this.