Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Problem/Motivation
D7 had a preserve_files option to trigger this behavior, we don’t have that in D8. This results in really slow migration when you're talking about multiple gigabytes of files.
Proposed resolution
Add a 'reuse' file configuration option. If that flag is set on the file migration, it will attempt to re-use files that are already in the destination with the same name.
Remaining tasks
Code a patch
Review
User interface changes
API changes
Additional process plugin option for file migrations to configure a 'preserve_files' flag.
Data model changes
Comment | File | Size | Author |
---|---|---|---|
#35 | file_migrations-2706405-35.patch | 4.08 KB | heddn |
#35 | interdiff_27-35.txt | 2.07 KB | heddn |
Comments
Comment #2
edysmpPatch for configuration option.
Comment #3
edysmpAdded tests.
Comment #4
benjy CreditAttribution: benjy at PreviousNext commentedComment #5
Anonymous (not verified) CreditAttribution: Anonymous commentedComment #6
benjy CreditAttribution: benjy at PreviousNext commentedComment #7
mikeryanThis will need a reroll when #2695297: Refactor EntityFile and use process plugins instead gets in.
Comment #9
heddnReady for more work now.
Comment #10
mikeryanI think "preserve_files" is going to be confusing to people who've used the D7 migrate module, because what it's doing here is not what is was doing there. In D7, "preserve_files" meant to not delete files on rollback (so that rolling back nodes with file references would not remove files when file_usage would otherwise be empty for a given file). The D7 means of indicating the desired behavior here (if the destination file already exists, use the existing file instead of recopying it) was to extend the core $replace flags of FILE_EXISTS_RENAME and FILE_EXISTS_REPLACE with an additional MigrateFile::FILE_EXISTS_REUSE.
Comment #11
heddnI have some logic I've included in #2795879: FileBlob process plugin that I wonder if we should pull into core and solve this more elegantly. We already have
FILE_EXISTS_ERROR
, why not just use it and not copy if the file already exists.Comment #12
ldavidsp CreditAttribution: ldavidsp at MTech, LLC commentedNo interdiff since this takes a different approach now that #2695297: Refactor EntityFile and use process plugins instead is in core.
Comment #14
heddnQuestion: why is the class under test named FileCopy and the test named CopyFile?
Comment #15
heddnIgnore #14.
Comment #18
heddnComment #20
heddnTrying this.
Comment #22
benjy CreditAttribution: benjy at PreviousNext commentedComment #23
heddnComment #25
heddnNone of the other tests seem to clean-up after themselves. So for now I'm going to punt on that and leave it to a follow-up if we decide that cleaning up files in the teardown is a necessary step.
Comment #27
heddnComment #29
heddnThis is ready for review.
Comment #30
phenaproximaI believe the public stream wrapper is already registered by the system module in a kernel test -- does it still pass if we remove this line?
Looks like this isn't adding anything useful, can we remove it?
Why use
self::
? Can we switch to$this->assertGreaterThan()
and$this->assertLessThan()
when asserting against the timestamp?Let's also add a couple of calls like
$this->assertInternalType('int', $modified_time)
, since, as far as I can infer from the PHP documentation, SplFileInfo::getMTime() might return FALSE on failure.Additionally, it'd be cool if we also added a test of an unsuccessful reuse.
Comment #31
edysmpFor #30
Comment #33
heddnLet's flip expected with actual. That should work better.
Comment #35
heddnPasses on local.
Comment #36
mikeryanNit: comma after (default) (can be fixed on commit).
I kinda got an icky feeling at adding a second to test time - but, I don't see an alternative and besides, if I'm counting correctly, the full test suite is already losing 22 seconds to sleep() calls...
Comment #37
catchCommitted/pushed to 8.3.x and cherry-picked to 8.2.x. Thanks!